Conversation
Copied over info from .mention-bot
|
🤔 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.
.github/CODEOWNERS
Outdated
| @@ -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?
.github/CODEOWNERS
Outdated
| 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
.github/CODEOWNERS
Outdated
| @@ -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.
|
@tiran since he would be the inaugural user of the file. 😄 |
|
mention-bot deletion is in #2923 :) |
|
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 |
|
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. :)
|
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/ |
|
Reading codeowner docs, it |
|
@Mariatta the |
|
@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 |
|
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.
|
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. |
|
In the current |
|
I added some comments on top of the file. |
|
I think for now we are only notifying the owner/expert listed in |
|
Not sure if people want to be requested for review for backport PRs? I removed |
|
😞 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 😛 |
|
To answer @Haypo : it only blocks PRs if you check a certain box and I'm never going to check that box (trust me 😉 ) |
|
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