-
Notifications
You must be signed in to change notification settings - Fork 76
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
Add README.md to the ladder/teams directory explaining how team management works #198
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is well written and I think captures the vast majority of what I'd expect out of reviewer teams. I'd like to encode a little bit more of a sense that code owners are responsible for the quality of assigned code areas including not only reviews but also regular maintenance and keeping tests reliable for their areas. But these aspects can probably also be extended onto this as a follow-up.
I had one small suggestion for mentors which is a new concept for the community.
Okay, I've made some edits based on feedback of the initial commit. So if everyone with eyeballs on this can take a fresh read from the top that would be lovely. I'm going to wait and do a commit squash after its clearer i've addressed what I can. There's undoubtly more typos i'm gonna brown bag fix 🤦♂️ |
ab61647
to
8b0c099
Compare
I've squashed back to a single commit in anticipation of possible approval and merge. |
Integrated minor suggestions for @xmulligan and resquashing. |
…ement works Signed-off-by: Jef Spaleta <[email protected]>
748b24a
to
40a750d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more wording improvement suggestions.
I think that mixing roles into this in the current state just makes the document more confusing than necessary. Maybe as a follow-up we can look at the roles and consider whether the "mentor" is a role as well, and see what information might be useful to integrate into this doc.
Other than that I just saw a couple of pretty minor wording / spelling nits that could be improved.
The Cilium project makes use of GitHub teams to help enforce responsibility boundaries with regard to the project codebase here on GitHub. | ||
GitHub teams loosely map groups of [contributors](https://github.com/cilium/community/blob/main/CONTRIBUTOR-LADDER.md) to the Cilium Project as either reviewers and/or committers, focused on a portion of the codebase. [GitHub teams](https://github.com/orgs/cilium/teams) are an implementation detail in how code access and review responsibilities are organized and are not a 1 to 1 mapping to [Cilium Project teams](https://github.com/cilium/community/blob/main/CONTRIBUTOR-ROLES.md). | ||
|
||
For example, the Cilium Project Code team, may require multiple GitHub teams in the Cilium GitHub organization to self-organize their work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice the "Cilium Project Code team" is not really something we actively maintain; it's more of a virtual designation for a category of contributors. That role just explains some expectations for some contributors, specifically committers who actively work on code, but we don't track a list of these anywhere or assign special permissions or responsibilities.
Overall I think this doc would probably benefit from just ignoring roles to start off with because roles are for the most part not encoded in these teams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To expand on the above: Roles are designated in the roles/
directory as markdown files, and aren't part of this directory or any of the corresponding automation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added to address @xmulligan suggestion to tie this back to roles language.
Since this directory is in fact under the ladder directory and github has no concept of ladder in its implementation language. There an ambiguity around the word "team" that is both a high level project concept and a github implementation specific concept.
In effect what we are really doing is using a whole bunch of GitHub subteams, teams within teams, to implement permission boundaries that make sense for our contributor ladder divisions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't realize there was review discussion about that.
Reading through the text, I don't think we need an example at this stage of the explanation, this specific sentence about the code team removes clarity rather than adding clarity. I'll post some suggested amended text to tie in the rest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see "ladder" and "teams" as somewhat orthogonal concepts. There is the general ladder of how we see different contributor's level of involvement in the project. Then for the most part when someone becomes a reviewer or higher on the ladder, they gain the privilege to join a specific team with specific permissions.
I'd probably put the ladder in one directory and teams under a different top-level directory if the directory hierarchy is creating confusion here.
|
||
Cilium GitHub organization team memberships can be self-managed by making use of team membership configuration files in this directory. | ||
|
||
Each Cilium GitHub team can establish a file in this directory managed by the CODEOWNERS system with the assistance of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each Cilium GitHub team can establish a file in this directory managed by the CODEOWNERS system with the assistance of | |
Each Cilium GitHub team can establish a file in this directory managed by the CODEOWNERS system with the assistance of |
It's a bit odd to state that a team can create a file in this directory. The team doesn't exist until the file exists in this directory.
I wonder if this particular case would be better documented under a new section for creating and deleting teams. Then we could also expand on the expectations for these. This PR can help provide some insights into that: #182 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to also state that creating / removing teams could be a separate addition to the doc. I think that this PR is already (a) documenting what exists + (b) introducing the mentor concept, so it would be easier to get the core wording in place first and then iterate on top as a separate PR for stuff like team changes (which is a much rarer case in practice, though we perhaps should revisit this more frequently)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually not true. The team can exist.. in the private full team management configuration.
So i'm envisioning initial team creation being still github org admin tasking, because you'll probavly need to touch the base team management config with potentially an initial group of members in the base config and then CODEOWNERS config for that team gets setup as part of that.
And then that team becomes self-service with the introduction of the file in this directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would simplify this doc if we only inform readers how to create a new team. The case where there's some existing team that maybe exists somewhere but isn't defined by this repo should be the exception and people involved in those teams should already be well aware of how that works (or at least communicating frequently with the core maintainers).
Co-authored-by: Joe Stringer <[email protected]> Signed-off-by: Jef Spaleta <[email protected]>
Co-authored-by: Joe Stringer <[email protected]> Signed-off-by: Jef Spaleta <[email protected]>
Co-authored-by: Joe Stringer <[email protected]> Signed-off-by: Jef Spaleta <[email protected]>
Co-authored-by: Joe Stringer <[email protected]> Signed-off-by: Jef Spaleta <[email protected]>
For example, the Cilium Project Code team, may require multiple GitHub teams in the Cilium GitHub organization to self-organize their work. | ||
These multiple GitHub teams will be composed of contributors at different steps along the contributor ladder: organizational members, reviewers, and committers. | ||
The different GitHub teams have different permissions, corresponding to the level of code access needed by the members of that team. | ||
|
||
Cilium GitHub organization team memberships can be self-managed by making use of team membership configuration files in this directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, the Cilium Project Code team, may require multiple GitHub teams in the Cilium GitHub organization to self-organize their work. | |
These multiple GitHub teams will be composed of contributors at different steps along the contributor ladder: organizational members, reviewers, and committers. | |
The different GitHub teams have different permissions, corresponding to the level of code access needed by the members of that team. | |
Cilium GitHub organization team memberships can be self-managed by making use of team membership configuration files in this directory. | |
Cilium GitHub organization team memberships can be self-managed by making use of team membership configuration files in this directory. | |
These teams will be composed of contributors at different steps along the contributor ladder: organizational members, reviewers, and committers. | |
The different GitHub teams have different permissions, corresponding to the level of code access needed by the members of that team. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking the last sentence here begs some more questions about what those permissions are and how they're configured, but I don't think that's critical to resolve right now. (That said the rest of the suggested changes above make sense to apply today).
i'll be trying to refactor this again today. |
Adding a readme to explain how to use the teams directory to self manage team members and team mentors.
team mentors are a new concept, meant to aid streamlining some of the team management tasks.. specifically making it possible to teams to self-manage which of their members are particpating in the automatic PR review assignment rotation.