-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
maintainers: add github user IDs in service to NixOS/rfcs#39 #66361
Conversation
Aw man, really disappointed that github allows reuse of usernames. This can make user links in commit messages very misleading. This PR wouldn't be needed if GitHub didn't allow that. |
dbf060a
to
2068c8f
Compare
I updated the recorded GitHub account for @langston-barrett, @moredhel, @fleaz, @furrycatherder, and @tobim by finding the commit which added them to the maintainer list, and using the new GitHub username associated with their account. This leaves two maintainers I couldn't track down, Edit: this eliminates all of the errors in the PR body. A new run is clean. |
I think we should go ahead with this. There is indeed the risk that the username (and thereby the ID that @grahamc found) corresponds now to a different user, however the impact such users can have is limited to the GitHub "Read" permission level or possibly later "Triage" level. In my opinion such a risk is acceptable. |
The change is correct in my case, at least. Thanks for taking care of this 🙂 |
It is correct in my case as well, sorry for not taking care of that. |
Thanks @FRidh, I agree. I'm going to do a bit more looking in to the IDs I can't be 100% sure of, but let's plan on merging this (or a regeneration of this PR) today. |
The original commit adding all the IDs was +1,118. I've deleted that commit, replacing it with one adding the github ID parts to the template only. I started to regenerate the list with only IDs I'm totally confident on, but hit the rate limit which I think means the universe is telling me to do at least 37 minutes of housework. |
Use the graphql api, generally speaking you get more out of it :) |
For each contributor's github handle, get that account name's GitHub ID. Then, «git blame» the maintainer file and ensure GitHub agrees the commit which added the maintainer to the list was authored by the GitHub ID we've recorded.
The new commit adding GitHub IDs has 1,019 additions. I'm inclined to leave the remaining ~100 un-updated for now. |
Note I've verified these names by:
|
Motivation for this change
Begin implementing NixOS/rfcs#39 : unprivileged maintainer team.
Step one: Track user's GitHub IDs alongside their github username (re: https://github.com/NixOS/rfcs/blob/master/rfcs/0039-unprivileged-maintainer-teams.md#changes-to-maintainersmaintainer-listnix)
This list is generated by taking the usernames in the list and looking up their ID, via https://github.com/grahamc/rfc39/blob/master/src/main.rs#L79
There is a question of have any of these usernames changed hands? Hard to verify. I can tell you these errors do occur:
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @