Skip to content
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions rfcs/0053-pull-request-workflow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
---
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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my experience "merger" usually means the resulting of a merge and rarely designate a person. The standard terms that could be used instead are "integrator" and "committer" (the second being less clearly related to PRs but clearer about the required privileges).


The responsibilities of theses roles are defined in the rest of this RFC.
Comment thread
mrVanDalo marked this conversation as resolved.
Outdated

## 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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a common practice that the merger, will backport changes without having a backport pull request.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull-request is created in by the contributor in the 5th line, right below the bar.


## 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.
* the [NixOS/backports team](https://github.com/orgs/NixOS/teams/backports)
**MUST** be informed about every Backport.
Comment thread
mrVanDalo marked this conversation as resolved.
Outdated
* after the Pull-Request to `master`, `staging` or `staging-next` is merged,
the Backport Pull-Request is created
* 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.

### Modules

* modules **SHOULD** have tests
* reviewers **SHOULD** encourage contributors to write tests for new modules
* modules **SHOULD NOT** be Backported

## 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)

# 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random idea: how about something like @ofborg backport 19.09?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is the bot would create the pull-request and the contributor must be able to edit it.
There are two backport possibilities

  • cherry pick
  • new pull-request, because the software differs in the stable branches.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the main catch is what happens if person A creates a PR and person B requests a backport…

* 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?"
Comment thread
mrVanDalo marked this conversation as resolved.
Outdated
* Add a link to this document in the
[Contribution Guidelines](https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md)
1 change: 1 addition & 0 deletions rfcs/0053-pull-request-workflow/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.history
Loading