From 1ceb4b37383f97b9e31bf037d9724e6c35cc6f74 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 16 Jan 2019 12:15:21 -0500 Subject: [PATCH 1/4] RFC-0039: unprivileged maintainer team --- rfcs/0039-unprivileged-maintainer-teams.md | 176 +++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 rfcs/0039-unprivileged-maintainer-teams.md diff --git a/rfcs/0039-unprivileged-maintainer-teams.md b/rfcs/0039-unprivileged-maintainer-teams.md new file mode 100644 index 000000000..4b883cc50 --- /dev/null +++ b/rfcs/0039-unprivileged-maintainer-teams.md @@ -0,0 +1,176 @@ +--- +feature: unprivileged-maintainer-teams +start-date: 2019-01-16 +author: Graham Christensen +co-authors: zimbatm +related-issues: https://github.com/NixOS/ofborg/pull/303 +--- + +# Summary +[summary]: #summary + +Package maintainers who are not able to commit directly to Nixpkgs +don't have adequate tools to attentively maintain their package. +OfBorg requests reviews of maintainers it can identify. GitHub only +allows requesting a review of a Collaborator of the repository. + +This RFC bridges that gap, and allows OfBorg to request reviews of +maintainers. + +# Motivation +[motivation]: #motivation + +The goal of this RFC is to involve package maintainers in reviewing +pull requests against their packages. This RFC does not grant +maintainers the ability to merge pull requests against their own +package. + +Maintainers take a responsibility for their package, and want to know +about updates to their package's expression. However, Nixpkgs receives +over 1,000 pull requests each month and subscribing to them all is not +a reasonable requirement to maintain a package. + +The ideal outcome is package maintainership means a more active role +in reviewing and approving changes to Nixpkgs. + +# Detailed design +[design]: #detailed-design + +Package maintainers will be a member of a GitHub team, allowing OfBorg +to request a review. + +## The Team + +We will create a GitHub team under the NixOS GitHub organization +called "Nixpkgs Maintainers" which only grants "read" access to +Nixpkgs. + +This team will not grant any privileges to the Nix ecosystem +repositories which non-members already have. They will not be able to +close other people's issues or PRs or push branches. Experimentation +and documentation shows this will only grant access to a team +discussion board on GitHub. + +Being a member of this team will let the user mark themselves as a +public member of the organization. This will show the NixOS logo on +their GitHub profile, and people will see "Member" next to their +account name when browsing issues. + +In order to be a member, each user will enable 2FA on their GitHub +account, since [the GitHub organization requires 2FA of all +members](https://github.com/NixOS/nixpkgs/issues/42761). + +See +https://help.github.com/articles/permission-levels-for-an-organization/ +for more information about what this will grant. + +## Changes to `maintainers/maintainer-list.nix` + +The existing Nixpkgs maintainer list already contains a structured +attribute set of per-maintainer details, including GitHub account +names. Automation will sync this list of GitHub handles with the +team's membership, automatically adding and removing people as the +list changes. + +GitHub handles can change from one user to another, and so we will +change the maintainer list to include the GitHub user *ID* as well as +their handle. When syncing, the automation will validate the user ID +matches. GitHub User IDs are easily found at +`https://api.github.com/users/«username»`. + +If a user ID's GitHub handle changes, the maintainer should remain +part of the team under their new name. The repository's nickname +should be updated to reflect their new nickname. + +## Team Automation + +The team must be automatically updated at least once a day to ensure +the maintainer list is fresh and up to date. The automation for this +will be written in Rust with the hubcaps library. It will run on the +NixOS infrastructure with limited credentials, with only sufficient +permission to manage the team. + +The automation will fetch a fresh version of Nixpkgs, extract the +maintainer information, and update the team. It will support a dry-run +option. + +New members of the team will receive an invitation to join the GitHub +organization. + +## OfBorg changes + +OfBorg will identify PRs which are approved by their maintainers, and +add a special label `approved-by-maintainer`. + +## Roll-Out Plan + +1. Write an explanatory post on Discourse about the what-and-why of + this plan. +2. Select a small group of maintainers who are not committers to be + part of the first round, and manually run the tooling, and pause + half a week to see what changes. +3. Automate the tooling on the infrastructure. +4. Expand the group to one quarter of the maintainers, and pause a + half a week to gauge response. +5. Expand the group to one half of the maintainers and wait one week. +6. Expand the group to all of the maintainers. + +If we receive no major feedback or problems during the rollout, we +will continue to 100%. + +# Drawbacks +[drawbacks]: #drawbacks + + - Putting each maintainer in a read only team will display + maintainers as "member", without specifying which team they are a + member of. This gives the impression of authority which maintainers + don't already receive. This is a pro and a con. + + - A mistake in the automation, or in the admin panel of GitHub could + grant the team write access to Nix ecosystem repositories. + + - Package maintainers who do not wish to have a GitHub account will + not benefit from this change. + + - Package maintainers who do have a GitHub account, but do not wish + to use 2 factor authentication will not benefit from this change. + + - If a GitHub user is a real jerk on the internet, it can potentially + reflect negatively on the NixOS ecosystem. Someone who is banned + from the NixOS GitHub organization is not allowed to be a package + maintainer. + +# Alternatives +[alternatives]: #alternatives + +Mentioning people in GitHub comments is the main alternative. This has +the major down-side of not receiving the support of [GitHub's UI +for requested reviews](https://github.com/pulls/review-requested). + + +# Unresolved questions +[unresolved]: #unresolved-questions + + - Is it possible for the automation to spam a user who doesn't want + to be part of the team with invitations? + - Do maintainers want to be part of this team? + - Will the requirement of 2FA cause a significant number of people to + not want to participate? + - How will we handle people who have been invited, but have not + accepted the invitation? + +# Future work +[future]: #future-work + + - Writing the automation program. + - Adding UIDs to every maintainer. + - Creating the GitHub team + - Updating the NixOS Org Configurations repository to run the + automation with credentials on an automated basis. + +# Future Potential RFCs +The following topics are explictly _not_ part of this RFC. + + - Allowing maintainers to merge pull requests against their packages + without having commit access. + - Requiring all maintainers to have a GitHub account with 2FA. From fd731f7ed559ea922a16eeb7ca0d5b61fd4651b9 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 16 Jan 2019 13:19:45 -0500 Subject: [PATCH 2/4] Addressing feedback --- rfcs/0039-unprivileged-maintainer-teams.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rfcs/0039-unprivileged-maintainer-teams.md b/rfcs/0039-unprivileged-maintainer-teams.md index 4b883cc50..45b79f391 100644 --- a/rfcs/0039-unprivileged-maintainer-teams.md +++ b/rfcs/0039-unprivileged-maintainer-teams.md @@ -46,7 +46,7 @@ called "Nixpkgs Maintainers" which only grants "read" access to Nixpkgs. This team will not grant any privileges to the Nix ecosystem -repositories which non-members already have. They will not be able to +repositories which non-members don't already have. They will not be able to close other people's issues or PRs or push branches. Experimentation and documentation shows this will only grant access to a team discussion board on GitHub. @@ -56,8 +56,8 @@ public member of the organization. This will show the NixOS logo on their GitHub profile, and people will see "Member" next to their account name when browsing issues. -In order to be a member, each user will enable 2FA on their GitHub -account, since [the GitHub organization requires 2FA of all +In order to be a member, each user will need to enable 2FA on their +GitHub account, since [the GitHub organization requires 2FA of all members](https://github.com/NixOS/nixpkgs/issues/42761). See @@ -69,8 +69,8 @@ for more information about what this will grant. The existing Nixpkgs maintainer list already contains a structured attribute set of per-maintainer details, including GitHub account names. Automation will sync this list of GitHub handles with the -team's membership, automatically adding and removing people as the -list changes. +team's membership, automatically adding and removing people to/from +the team as the list changes. GitHub handles can change from one user to another, and so we will change the maintainer list to include the GitHub user *ID* as well as @@ -79,8 +79,8 @@ matches. GitHub User IDs are easily found at `https://api.github.com/users/«username»`. If a user ID's GitHub handle changes, the maintainer should remain -part of the team under their new name. The repository's nickname -should be updated to reflect their new nickname. +part of the team under their new handle. The user's entry in +`maintainer-list.nix` should be updated to reflect their new handle. ## Team Automation From 19f6d2f3f1d2d1477b1f27d80a60a6634540ea2e Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Thu, 28 Feb 2019 14:45:17 -0500 Subject: [PATCH 3/4] Incorporate RFC Shepherd meeting feedback, and clarify that maintainers should use reviews. --- rfcs/0039-unprivileged-maintainer-teams.md | 32 ++++++++++++++++------ 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/rfcs/0039-unprivileged-maintainer-teams.md b/rfcs/0039-unprivileged-maintainer-teams.md index 45b79f391..2d2dac1b2 100644 --- a/rfcs/0039-unprivileged-maintainer-teams.md +++ b/rfcs/0039-unprivileged-maintainer-teams.md @@ -70,7 +70,7 @@ The existing Nixpkgs maintainer list already contains a structured attribute set of per-maintainer details, including GitHub account names. Automation will sync this list of GitHub handles with the team's membership, automatically adding and removing people to/from -the team as the list changes. +the team as the master branch's maintainer list changes. GitHub handles can change from one user to another, and so we will change the maintainer list to include the GitHub user *ID* as well as @@ -90,13 +90,19 @@ will be written in Rust with the hubcaps library. It will run on the NixOS infrastructure with limited credentials, with only sufficient permission to manage the team. -The automation will fetch a fresh version of Nixpkgs, extract the -maintainer information, and update the team. It will support a dry-run -option. +The automation will fetch a fresh version of Nixpkgs's master branch, +extract the maintainer information, and update the team. It will +support a dry-run option. New members of the team will receive an invitation to join the GitHub organization. +## Changes to Reviewer/Maintainer Behavior + +Reviewers and maintainers should use GitHub's review tools (Approve, +Request Changes, etc.) to clearly communicate their feedback about the +pull request. + ## OfBorg changes OfBorg will identify PRs which are approved by their maintainers, and @@ -135,10 +141,8 @@ will continue to 100%. - Package maintainers who do have a GitHub account, but do not wish to use 2 factor authentication will not benefit from this change. - - If a GitHub user is a real jerk on the internet, it can potentially - reflect negatively on the NixOS ecosystem. Someone who is banned - from the NixOS GitHub organization is not allowed to be a package - maintainer. + - Someone who is banned from the NixOS GitHub organization is not + allowed to be a package maintainer. # Alternatives [alternatives]: #alternatives @@ -148,6 +152,18 @@ the major down-side of not receiving the support of [GitHub's UI for requested reviews](https://github.com/pulls/review-requested). +# Reesolved questions +[resolved]: #reesolved-questions + + - Is it possible for the automation to spam a user who doesn't want + to be part of the team with invitations? + No. + - Do maintainers want to be part of this team? + - Will the requirement of 2FA cause a significant number of people to + not want to participate? + - How will we handle people who have been invited, but have not + accepted the invitation? + # Unresolved questions [unresolved]: #unresolved-questions From 1a0f5b6536adb398c91edcd166b9d576345e1aa9 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Thu, 28 Feb 2019 15:40:57 -0500 Subject: [PATCH 4/4] fixup: resolved question duplication --- rfcs/0039-unprivileged-maintainer-teams.md | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/rfcs/0039-unprivileged-maintainer-teams.md b/rfcs/0039-unprivileged-maintainer-teams.md index 2d2dac1b2..bcdbb82cc 100644 --- a/rfcs/0039-unprivileged-maintainer-teams.md +++ b/rfcs/0039-unprivileged-maintainer-teams.md @@ -152,23 +152,16 @@ the major down-side of not receiving the support of [GitHub's UI for requested reviews](https://github.com/pulls/review-requested). -# Reesolved questions -[resolved]: #reesolved-questions +# Resolved questions +[resolved]: #resolved-questions - Is it possible for the automation to spam a user who doesn't want to be part of the team with invitations? No. - - Do maintainers want to be part of this team? - - Will the requirement of 2FA cause a significant number of people to - not want to participate? - - How will we handle people who have been invited, but have not - accepted the invitation? # Unresolved questions [unresolved]: #unresolved-questions - - Is it possible for the automation to spam a user who doesn't want - to be part of the team with invitations? - Do maintainers want to be part of this team? - Will the requirement of 2FA cause a significant number of people to not want to participate?