Skip to content

let anyone in the org approve PRs to Forge #1839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented May 23, 2025

forge is for everyone, so don't restrict it. in any case, there is very little downside to "eagerly" merging documentation prs, since it's easy to fix them in followups and they can't cause nightly breakage.

cc #council > who maintains cross-team documentation?, #t-infra > default writable repos @ 💬

forge is for everyone, so don't restrict it. in any case, there is very
little downside to "eagerly" merging documentation prs, since it's easy
to fix them in followups and they can't cause nightly breakage.
edition = "maintain"
release = "maintain"
triagebot = "maintain"
all = "maintain"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also change this to write, especially now that we give access to more people. I don't think that maintain is needed for Forge.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the difference between the two?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit hard to summarize 😅 Here's a table: https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/managing-repository-roles/repository-roles-for-an-organization

With maintain, you can modify wikis and do a bunch of additional things, but for modifying issues/labels and merging/approving PRs, write should be enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh interesting, the main difference I see is that "maintain" lets you push to protected branches. I suppose we were already letting people bypass review, it's just that nobody noticed lol.

i'll change this to "write".

Copy link

github-actions bot commented May 23, 2025

Dry-run check results

[WARN  sync_team] sync-team is running in dry mode, no changes will be applied.
[INFO  sync_team] synchronizing github
[INFO  sync_team] 💻 Team Diffs:
    📝 Editing team 'rust-lang/all':
      Adding member 'arshiamufti' with member role
      Adding member 'graciegregory' with member role
    📝 Editing team 'rust-lang/community-rustbridge':
      Adding member 'arshiamufti' with member role
    📝 Editing team 'rust-lang/community-survey':
      Adding member 'graciegregory' with member role
    📝 Editing team 'rust-lang/wg-triage':
      Adding member 'edmilsonefs' with member role
    💻 Repo Diffs:
    📝 Editing repo 'rust-lang/rust-forge':
      Permission Changes:
        Giving team 'all' write permission
        Removing team 'leadership-council''s maintain permission 
        Removing team 'crates-io''s maintain permission 
        Removing team 'lang-ops''s maintain permission 
        Removing team 'triagebot''s maintain permission 
        Removing team 'lang''s maintain permission 
        Removing team 'community''s maintain permission 
        Removing team 'docs-rs''s maintain permission 
        Removing team 'libs-api''s maintain permission 
        Removing team 'libs''s maintain permission 
        Removing team 'release''s maintain permission 
        Removing team 'compiler''s maintain permission 
        Removing team 'edition''s maintain permission 

@traviscross
Copy link
Contributor

I'm not sure this PR is correct. While the forge is for everyone, and we give access to teams that ask, my understanding is that we set up specific things when doing so, and so we currently do prefer the teams to ask.

cc @ehuss

@jyn514
Copy link
Member Author

jyn514 commented May 23, 2025

do you know what the "specific things" are?

@lolbinarycat
Copy link
Contributor

does "all" also include working groups? wg-triage has a lot of docs there, and it would be nice for us to be able to easily modify them.

@steffahn
Copy link
Member

does "all" also include working groups?

@lolbinarycat the answer appears to be "no" ;-)

team/teams/all.toml

Lines 1 to 7 in 3dc6c7c

name = "all"
kind = "marker-team"
[people]
leads = []
members = []
include-all-team-members = true

team/src/schema.rs

Lines 296 to 307 in 3dc6c7c

if self.people.include_all_team_members {
for team in data.teams() {
if team.kind != TeamKind::Team
|| team.name == self.name
// This matches the special alumni team.
|| team.is_alumni_team()
{
continue;
}
members.extend(team.members(data)?);
}
}

team/src/schema.rs

Lines 131 to 136 in 3dc6c7c

pub(crate) enum TeamKind {
Team,
WorkingGroup,
ProjectGroup,
MarkerTeam,
}

@jyn514
Copy link
Member Author

jyn514 commented May 28, 2025

that seems like an oversight to me, I would expect this to include everyone in the organization except for alumni

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants