Repository navigation
@nodejs team permissions #895
Description
Tip
Several nodejs teams are referenced in this issue, but they are not notified because the following snippet is used:
**@[]()nodejs/team**This format does not ping the teams.
After @Trott highlighted some points in this comment, I reviewed the configuration of the @nodejs/... teams. Based on my findings, I believe two changes could significantly improve the situation:
- The @nodejs/all-members team does not actually include all members. Out of over 340 organization members, only 142 are part of this team.
- Currently, reviewers are pinged via comments instead of using GitHub's review feature because the all-members team lacks triage access to
nodejs/node.
I'm not proposing a major overhaul of the Node.js team system, but I suggest the following:
- All teams should be placed under @nodejs/all-members.
- @nodejs/all-members should be granted Triage permissions for
nodejs/node, similar to thenodejs/moderationrepo. This would allow the CODEOWNERS file to automatically request reviews. Most (if not all) members already have triage or higher access to the repository, just not at the all-members level.
Activity
[-]Organization of the @nodejs teams[/-][+]@nodejs team permissions[/+]benjamingr commentedon Jul 17, 2024
Please be mindful of pinging anyone in the org for an administrative issue like this
avivkeller commentedon Jul 17, 2024
No one was pinged (except for @Trott)
marco-ippolito commentedon Jul 17, 2024
I think everyone is pinged when you mention
@nodejsavivkeller commentedon Jul 17, 2024
I've tested it, and that's not the case (phew!)
Trott commentedon Jul 17, 2024
As demonstrated by the negative reaction to a single ping here that might not have even actually happened, it's going to be very much an uphill battle trying to get engagement and consensus on a significant overhaul of the org setup just so that people can get even more pings (or the same amount of pings but done differently). I think this is the wrong approach and that we should abandon CODEOWNERS, or at least greatly reduce the number of files and teams in the CODEOWNERS file.
I think the problem is three-fold:
I think the CODEOWNERS file was a well-intentioned experiment that good people put work into but, unfortunately, has not worked out. There's nothing wrong about that. Failed experiments are part of the process. If all experiments succeed, they're probably not really experiments.
I think people generally ignore the CODEOWNERS ping. I could be wrong, but I think it's mostly noise at this point and we should get rid of it or at least vastly reduce its scope. Most files do not need CODEOWNERS entries.
This is related to the fact that the typical team contains one or two active people and six or eight people that will never respond to a ping and aren't involved in the org anymore. The only real solution to that is for one or more TSC members to actively manage the teams, removing a few people (and maybe adding a few) every few days (because doing all the teams in one sitting is just not possible).
aduh95 commentedon Jul 17, 2024
I very much doubt it, GitHub docs says it's only when mentioning people or teams (https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#mentioning-people-and-teams), nothing about mentioning orgs. Also the notification I've received says
subscribed, notmentionnorteam mention.avivkeller commentedon Jul 17, 2024
I wouldn't call it significant, it's just moving teams under a single roof, and possibly changing the permissions of that house.
The benefit of a CODEOWNERS ping rather than the comment is that it doesn't subscribe them to the issue (IIRC). It lets them know they've been pinged, but doesn't subscribe them to future updates.
Whether or not CODEOWNERS is the best method to ping users, I think the comment is too much: it pings and subscribes users to the issue.
If that's the case, the bot ping can be removed, and the CODEOWNERS file deleted.
In my opinion, the order of most pings to least pings is
3. Bot comment
2. CODEOWNERS review
\1. None
If the ping was to be kept, I think doing it using GitHub reviews would be better, but if the ping was to be removed, than that can all be ignored.
The benefit of removing the ping entirely is that organization members monitor the issues, so if someone needed to be pinged, it could be done manually by members.
However, I still think all teams should fall under the all-members team, for ease-of-management.
And, if the all-members team is given triage access (or explicit read might work), members can request reviews from teams when needed.
aduh95 commentedon Jul 17, 2024
Ideally getting notified when some files are modified would be a feature of GitHub, so folks do not need to ask for someone to add them to a team (and remove them when they no longer want the notifications), and we do not need to manage teams at all (except those who give write access to some repo). Having said that, I realize it's convenient to be able to ping teams in e.g. issues, and we might miss having teams in those cases.
Realistically, we have teams, and have to use them (and we the TSC should do a better job at managing them).
Regarding the proposal of giving triage permission to
all-members: -1, that would force us to either gate-keep joining ping-only teams, and force us to "clean up" inactive members.Regarding moving more teams to
all-members: why? why do you care? why should we care?Regarding replacing bot pings with bot requesting reviews: why not, that seems relatively easy to do, I've seen it done on other open-source repo, I'd say it's a good idea, let's go for it (but it's not an issue for
admin, it only requires someone to open a PR).Regarding giving the possibility to request review from teams instead of individuals: meh, not worth it IMO.
avivkeller commentedon Jul 17, 2024
The reason this is an issue for admin, is because all teams need to be under all-members (or each team needs to have added permissions) for this to work. Once all teams are under all-members, that team needs to be given triage permissions (which is the minimum needed for reviews on teams)
aduh95 commentedon Jul 17, 2024
No, it can work by requesting reviews from individual instead of teams. The bot can check the CODEOWNERS file, list the members of the listed teams, and request reviews from them instead of pinging the team.
avivkeller commentedon Jul 17, 2024
Got it. I can open a PR later today. Feel free to close this if you think it's resolved, but otherwise feel free to keep this open.
avivkeller commentedon Jul 17, 2024
Sorry for the noise, I've moved this to nodejs/node#53897
avivkeller commentedon Jul 17, 2024
Closing in favor for discussion there.