diff --git a/rfcs/0053-pull-request-workflow.md b/rfcs/0053-pull-request-workflow.md new file mode 100644 index 000000000..c0066297e --- /dev/null +++ b/rfcs/0053-pull-request-workflow.md @@ -0,0 +1,127 @@ +--- +feature: pull-request workflow +start-date: 2019-09-28 +author: Ingolf Wagner (@mrVanDalo) +co-authors: lassulus (@lassulus) +shepherd-team: (names, to be nominated and accepted by RFC steering committee) +shepherd-leader: (name to be appointed by RFC steering committee) +related-issues: +--- + +# Summary +[summary]: #summary + +Pull request on GitHub are the main way we improve our code base. +This document should settle everything that needs to be done +to bring code upstream in the +[Nixpkgs Repository](https://github.com/nixos/nixpkgs/). + +# Motivation +[motivation]: #motivation + +Eliminate questions from contributors and maintainers +about what should be done next and who should do it. +This is not a new approach, it is more a settlement +on how we do it now. + +# Detailed design +[design]: #detailed-design + +Define all steps of a pull request. +Use Roles to define responsibilities in every step. + +Terms like **SHOULD** and **MUST** are defined in +[IETF RFC 2119](https://tools.ietf.org/html/rfc2119). + +## Roles +[roles]: #roles + +Everybody involved in the process of contributing has one or multiple +of the following roles + +* *Contributor* is the person proposing the pull request +* *Bot* is a bot that provides automated feedback +* *Reviewer* is any person that reviews the pull request + (for example a member of [NixOS/nixpkgs-maintainers](https://github.com/orgs/NixOS/teams/nixpkgs-maintainers)) +* *Merger* is any person with merge privileges + +The responsibilities of these roles are defined in the rest of this RFC. + +## States of a pull request +[state]:#states + +This diagram defines all states of a pull request, +and their transitions to other states. + +![pull-request state](0053-pull-request-workflow/pull-request-states.svg) + +## Responsibilities and Actions +[responsibilities]:#responsibilities + +The responsibilities of every role is defined by the following diagram: + +![pull-request activity](0053-pull-request-workflow/pull-request-roles.svg) + +## About pull requests + +### Packages + +* Contributors **SHOULD** evaluate and signal that a backport is necessary. +* The [NixOS/backports team](https://github.com/orgs/NixOS/teams/backports) + **SHOULD** be pinged in situations that are unclear. +* Backport pull requests **MUST** be linked to the original pull requests (using `git cherry-pick -x`). +* [NixOS/nixpkgs-maintainers](https://github.com/orgs/NixOS/teams/nixpkgs-maintainers) + and + [NixOS/backports](https://github.com/orgs/NixOS/teams/backports) + can deny the backport. +* After the pull request to `master`, `staging` or `staging-next` is merged, + the backport pull request is created + +### Modules + +* Modules **SHOULD** have tests +* Reviewers **SHOULD** encourage contributors to write tests for new modules +* Module changes **SHOULD NOT** be backported. + * For example a module change that's needed due to a package backport is a valid exception + +## What to backport + +* Security patches which aren't major updates +* If a security patch is a major upgrade, try and find patches to our + current version which accomplish the same goal. Apply the major + update to master, and the patches to stable. +* Bug fixes to applications which, again, aren't major updates. + Generally be cautious about these. +* Any updates when the current stable version is utterly broken. A + key example of this is Spotify, who regularly breaks their old + versions. +* Extremely security-sensitive software, in particular Chrome, + Chromium, Firefox, Thunderbird, and of course the kernel. + +### Don't backport if ... + +* the patch is just for Darwin, they use nixpkgs-unstable not a + stable branch. + +## Links + +* [How to write module tests](https://nixos.org/nixos/manual/index.html#sec-nixos-tests) +* [Contribution guidelines](https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md) +* [What to backport](https://gist.github.com/grahamc/c60578c6e6928043d29a427361634df6#what-to-backport) +* [RFC26 : staging workflow](./0026-staging-workflow.md) + +# Unresolved questions +[unresolved]: #unresolved-questions + +* The pull request of a backport should be created by the bot. + But if that is the case, the original contributor might not be able + to make changes on the branch behind the pull request. +* Backports without changes in `master` are not discussed. + for example security patches that only affect older versions in stable. + +# Future work +[future]: #future-work + +* The pull request template needs an option "backport needed?" +* Add a link to this document in the + [Contribution Guidelines](https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md) diff --git a/rfcs/0053-pull-request-workflow/.gitignore b/rfcs/0053-pull-request-workflow/.gitignore new file mode 100644 index 000000000..8f5fd87c3 --- /dev/null +++ b/rfcs/0053-pull-request-workflow/.gitignore @@ -0,0 +1 @@ +.history diff --git a/rfcs/0053-pull-request-workflow/pull-request-roles.graphml b/rfcs/0053-pull-request-workflow/pull-request-roles.graphml new file mode 100644 index 000000000..3ec03e7ba --- /dev/null +++ b/rfcs/0053-pull-request-workflow/pull-request-roles.graphml @@ -0,0 +1,672 @@ + + + + + + + + + + + + + + + + + + + + + + + Pull request, roles and steps + Contributor + + + + + + + Bot + + + + + + + Reviewer + + + + + + + Merger + + + + + + + master branch + + + + + + + stable branch + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + create pull request + + + + + + + + + + + + + + + + + builds? + + + + + + + + + + + + + + + + + update pull request + + + + + + + + + + + + + + + + + build changes + + + + + + + + + + + + + + + + + works? + + + + + + + + + + + + + + + + + merge pull request + + + + + + + + + + + + + + + + + backport? + + + + + + + + + + + + + + + + + create backport +pull request + + + + + + + + + + + + + + + + + builds? + + + + + + + + + + + + + + + + + update backport +pull request + + + + + + + + + + + + + + + + + DONE + + + + + + + + + + + + + + + + + + + + + + + build changes + + + + + + + + + + + + + + + + + works? + + + + + + + + + + + + + + + + + merge backport +pull request + + + + + + + + + + + + + + + + + START + + + + + + + + + + + + + + + + + + + + + + + + + + + no + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + yes + + + + + + + + + + + + + + + + + + + + + + + + + + + + no + + + + + + + + + + + + + + + + + + yes + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + no + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + no + + + + + + + + + + + + + + + + + + + + + + + + + + + + yes + + + + + + + + + + + + + + + + + + + + no + + + + + + + + + + + + + + + + + + + + + + + + + + + + yes + + + + + + + + + + + + + + + + + + + + + + + + + + + + yes + + + + + + + + + + + + + + + + diff --git a/rfcs/0053-pull-request-workflow/pull-request-roles.svg b/rfcs/0053-pull-request-workflow/pull-request-roles.svg new file mode 100644 index 000000000..8c5f1b3b0 --- /dev/null +++ b/rfcs/0053-pull-request-workflow/pull-request-roles.svg @@ -0,0 +1,238 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Pull request, roles and steps + Contributor + Bot + Reviewer + Merger + + + master branch + + + stable branch + + + + + + + create pull request + + + + + + + builds? + + + + + + + update pull request + + + + + + + build changes + + + + + + + works? + + + + + + + merge pull request + + + + + + + backport? + + + + + + + create backport + pull request + + + + + + + builds? + + + + + + + update backport + pull request + + + + + + + + + + + + + + DONE + + + + + + + build changes + + + + + + + works? + + + + + + + merge backport + pull request + + + + + + + + + + + + + + START + + + + + + + + no + + + + + + yes + + + + + + no + + + + yes + + + + + + yes + + + + + + no + + + + + + no + + + + yes + + + + + + no + + + + yes + + + + + + + diff --git a/rfcs/0053-pull-request-workflow/pull-request-states.graphml b/rfcs/0053-pull-request-workflow/pull-request-states.graphml new file mode 100644 index 000000000..888cc1d32 --- /dev/null +++ b/rfcs/0053-pull-request-workflow/pull-request-states.graphml @@ -0,0 +1,311 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Open + Contributor has done their work +Contributor has referenced all issues fixed by the pull request +Contributor has decided if the pull request needs a backport + + + + + + + + + + + + + + + + + + + Tested + Bot verified project builds +Bot verified that tests of the build are green + + + + + + + + + + + + + + + + + + + Reviewed + Reviewer verified that functionality of all affected packages + + + + + + + + + + + + + + + + + + + + Merged + Merger has merged the pull request to the master branch + + + + + + + + + + + + + + + + + + + Closed + Pull request is obsolete + + + + + + + + + + + + + + + + + + + Needs Correction + Contributor has to adjust the pull request + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/rfcs/0053-pull-request-workflow/pull-request-states.svg b/rfcs/0053-pull-request-workflow/pull-request-states.svg new file mode 100644 index 000000000..f97440c96 --- /dev/null +++ b/rfcs/0053-pull-request-workflow/pull-request-states.svg @@ -0,0 +1,211 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Open + Contributor has done their work + Contributor has referenced all issues fixed by the pull request + Contributor has decided if the pull request needs a backport + + + + + + + + + + Tested + Bot verified project builds + Bot verified that tests of the build are green + + + + + + + + + + Reviewed + Reviewer verified that functionality of all affected packages + + + + + + + + + + Merged + Merger has merged the pull request to the master branch + + + + + + + + + + Closed + Pull request is obsolete + + + + + + + + + + Needs Correction + Contributor has to adjust the pull request + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/rfcs/0053-pull-request-workflow/shell.nix b/rfcs/0053-pull-request-workflow/shell.nix new file mode 100644 index 000000000..d23a25d3c --- /dev/null +++ b/rfcs/0053-pull-request-workflow/shell.nix @@ -0,0 +1,16 @@ +with import {}; + +stdenv.mkDerivation rec { + + name = "rfc-53"; + + env = buildEnv { + name = name; + paths = buildInputs; + }; + + buildInputs = [ + yed + ]; + +}