Uh oh!
There was an error while loading. Please reload this page.
Create CODEOWNERS - #2924
Conversation
Copied over info from .mention-bot
Mariatta
commented
Jul 27, 2017
🤔 wondering if lowercase |
brettcannon
left a comment
There was a problem hiding this comment.
Might as well delete .mention-bot while we're at it.
| @@ -0,0 +1,11 @@ | |||
| Doc/library/hashlib.rst @tiran | |||
There was a problem hiding this comment.
Could this be collapsed with Lib/hashlib.py with hashlib.*? Same for ssl.*? And if you really want to catch everything including tests with one rule then would *hashlib.* work?
| Modules/_ssl* @tiran | ||
| Modules/hashlib.h @tiran | ||
| Python/pyhash.c @tiran | ||
| Include/pyhash.h @tiran |
There was a problem hiding this comment.
pyhash.* should cover both the .c and .h files.
There was a problem hiding this comment.
So I'm thinking to replace with the following patterns. What do you think? cc @tiran
# Hashing
**/*hashlib* @tiran
**/*pyhash* @tiran
# SSL
**/*ssl* @tiran
| @@ -0,0 +1,11 @@ | |||
| Doc/library/hashlib.rst @tiran | |||
There was a problem hiding this comment.
Since this file might grow a lot, we should probably have a comment per section to say what the rules are meant for, e.g. # Hashing.
brettcannon
commented
Jul 28, 2017
@tiran since he would be the inaugural user of the file. 😄 |
Mariatta
commented
Jul 28, 2017
mention-bot deletion is in #2923 :) |
brettcannon
commented
Jul 28, 2017
Then if @tiran is happy with the results and we add a section comment so we know thematically what the rules are meant to be for, then it LGTM to merge this and delete the |
tiran
commented
Jul 29, 2017
pitrou
commented
Jul 29, 2017
I really dislike the "owners" terminology as used here. There are "experts", which is the terminology used in the devguide. I personally don't want some people to start being possessive towards code they contributed (any more than they might already be). |
tiran
left a comment
There was a problem hiding this comment.
I'm happy to be the guinea pig for your experiments. :)
tiran
commented
Jul 29, 2017
Instead of assigning individual users to files, could we create teams and assign teams to files instead? @pitrou yes, I agree with you. The term "owner" is problematic. However we can't do much about it. It's a github standard, https://help.github.com/articles/about-codeowners/ |
Mariatta
commented
Jul 29, 2017
Reading codeowner docs, it |
tiran
commented
Jul 29, 2017
@Mariatta the |
brettcannon
commented
Jul 30, 2017
@pitrou Yes, the terminology doesn't line up with what we will use the file for, but it's dictated by GitHub (they actually have an option to say a pull request can't be merged until an owner listed in that file has been cleared, hence the naming). We could have a comment at the top of the file to point this fact out if you want. @tiran If we use a As for having teams, my plan was to get this file in first, and then solicit people to tell me what teams they wanted created as part of the announcement that the file now existed. I would then create the team and then make all team members admins for that team so they can control when they leave, adding others, etc. That means I only have to create the initial team and past that I don't have to maintain it. But one thing at a time. 😄 And I hope no one is silly enough to add a |
Mariatta
commented
Jul 30, 2017
I pushed the change. Please re-review. |
vstinner
left a comment
There was a problem hiding this comment.
I don't understand the purpose of this file. Please write a short description maybe pointing to a full description somewhere else.
Does it technically prevent merging any kind of change in these files without @tiran approval?
I see a risk of abusing this file.
vstinner
commented
Jul 31, 2017
https://help.github.com/articles/about-codeowners/ mentions adding automatically requests reviews. That's fine. My question is about the optional part: do you want to require @tiran review? Or later require someone else review on other parts of the code. |
Mariatta
commented
Jul 31, 2017
In the current |
Mariatta
commented
Jul 31, 2017
I added some comments on top of the file. |
Mariatta
commented
Jul 31, 2017
I think for now we are only notifying the owner/expert listed in |
Mariatta
commented
Jul 31, 2017
Not sure if people want to be requested for review for backport PRs? I removed |
Mariatta
commented
Aug 1, 2017
😞 I feel terrible that I made two personal branches in python/cpython repo that lasted longer than 24 hours. So I'm going to merge this now 😛 |
brettcannon
commented
Aug 1, 2017
To answer @Haypo : it only blocks PRs if you check a certain box and I'm never going to check that box (trust me 😉 ) |
vstinner
commented
Aug 1, 2017
via email
Brett: "I'm never going to check that box (trust me 😉 )"
Oh ok, that's fine in this case ;-) |
Copied over info from .mention-bot
python/core-workflow#160