Repository navigation
Add CODEOWNERS and update maintainer guidelines - #16690
Conversation
| @@ -0,0 +1,11 @@ | |||
| # See docs/maintainer-guide.md#subscribing-to-changes-with-codeowners | |||
| # Last matching pattern wins: list all subscribers to a path on one line. | |||
There was a problem hiding this comment.
I also just realize, maintainers could subscribe to changes to the contribution or maintainers guide with this.
I'm sure I've missed some updates in the past, and typeshed's best practices is constantly evolving.
For instance, we could do the following for every name found in this file:
docs/maintainer-guide.md @X @Y @Z
CONTRIBUTING.md @X @Y @Z
| and assume that the reader has no access to the diff. | ||
| * Delete branches for merged PRs (by maintainers pushing to the main repo). | ||
|
|
||
| ## Subscribing to Changes with CODEOWNERS |
There was a problem hiding this comment.
Style wise: There's already a mix of manual wrapping and no manual wrapping for this MD file.
Personally I prefer to never manually wrap. Most IDEs can easily be configured to auto-wrap only in MD files. GitHub also auto-wraps. Let readers use their full available screen width for text.
But CONTRIBUTING.md also mostly has manual wrapping around 80-90 chars width.
There was a problem hiding this comment.
We should probably at least run an auto formatter (e.g. prettier) over all our Markdown files. I'm not sure it's worth to add one to pre-commit.
There was a problem hiding this comment.
If you add one, may as well.
I'm a big fan of dprint. But maybe something else would better fit this project (especially given pre-commit isolation issues)
| * Delete branches for merged PRs (by maintainers pushing to the main repo). | ||
|
|
||
| ## Subscribing to Changes with CODEOWNERS | ||
|
|
There was a problem hiding this comment.
This section could go in CONTRIBUTING.md considering it can affect contributors. But I think it's more maintainer facing to know about the what/where/who/how of CODEOWNERS.
The part about "trusted contributors" could move or be duplicated. But I wanted to keep everything in one place.
| Trusted contributors who are actively involved with specific stubs (for example, the original author of those stubs) may request to be added by opening a PR that edits the file. This is limited to trusted and involved contributors, so that maintainers don't have to constantly edit the file. By requesting to be added, a contributor acknowledges that: | ||
|
|
||
| * They will be requested as a reviewer on every PR touching those stubs. | ||
| * Maintainers will not necessarily wait for their review before merging. | ||
| * Requests to be removed from the file also go through a PR, and may take some time to be merged. |
There was a problem hiding this comment.
Users and teams must have explicit
writeaccess to the repository
So I don't think this is going to work for trusted contributors unless we give them write access -- and I don't know why we'd do that without granting them the maintainer role
There was a problem hiding this comment.
Oh. If that's the case let's remove that part, yeah. Still a useful tools for maintainers who'd want specific updates.
There was a problem hiding this comment.
I suppose to reach the same effect for contributors, a GitHub action to automatically request reviews may be required.
There was a problem hiding this comment.
yeah, definitely not opposing the addition of the file for maintainers! Just think we should get rid of the guidance for third-party contributors, since it won't work unfortunately
There was a problem hiding this comment.
Yeah, I'm currently remote, on my phone. If you wanna do it I can immediatly approve.
I don't think anyone will oppose removing a faulty text that just got added.
Can add it back if we go the actions route. (Which is a bit more involved)
There was a problem hiding this comment.
What we did on CPython was to give triagers "write" access but use branch protection rules so they can't actually merge or create branches. That's also a bit confusing though so I'm not sure it's worth it.
There was a problem hiding this comment.
I think that made sense in CPython, where it's a much bigger project with many more maintainers and a dedicated "triagers" team that sits between the core devs and random OSS contributors in terms of trust level. I'm not sure it makes so much sense to add a dedicated "triagers" team over here
Closes #16663
Keeping this PR description light, as I want everything to be perfectly clear and unambiguous from the doc update alone.
Feel free to wordsmith.
VSCode extension for syntax highlighting:
