Skip to content

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
Member

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
Member

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

@jieyouxu jieyouxu added needs-team-repo-admin-review This change requires one of the `team-repo-admins` to review. S-has-concerns Status: has outstanding concerns that need to be addressed. T-leadership-council Relevant to the leadership council. labels Aug 8, 2025
@ehuss
Copy link
Contributor

ehuss commented Aug 8, 2025

I don't feel strongly about this, but my slight preference is to keep the current permissions as-is. Or, at least, to have a clearer statement about why this is being done. The frequency that new teams are added to the forge is very small, so the overhead of adding a new line to this file seems relatively small to me. I feel more comfortable keeping the permissions set to the teams who actually have a presence on the forge, to help make it clearer what people's responsibilities are.

@traviscross
Copy link
Contributor

I'd second Eric's inclinations here and propose to close this for now.

@jieyouxu jieyouxu removed the needs-team-repo-admin-review This change requires one of the `team-repo-admins` to review. label Aug 9, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Aug 9, 2025

Given the preferences indicated above, I'm going to close this as "status quo seems acceptable and explicitly writing out the teams might make it more obvious which teams have a prescence on Forge".

@jieyouxu jieyouxu closed this Aug 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-has-concerns Status: has outstanding concerns that need to be addressed. T-leadership-council Relevant to the leadership council.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants