Share your changes

The author of a pull request (PR) is responsible for its “life cycle”. He is responsible for contacting the various parties involved, following the review, responding to comments and correcting the code following review (you could also check dedicated page about code review).

In the case of a large PR, don’t hesitate to ask several reviewers to organize themselves, or even to carry out the review together, reviewers and author.

1. Open a pull request
   Once your changes are ready, you have to request integration with the dev branch.

   If possible:

   • Make PR of logical and atomic units too (avoid mixing refactoring, new features and bug fix at the same time).
   • Add a description to PRs to explain what they do and why.
   • Help the reviewer by following advice given in mtlynch article.
   • Add tags area:<affected_area> to show which part of the application have been impacted. It can be done through the web interface.

2. Take feedback into account
   Once your PR is open, other contributors can review your changes:

   • Any user can review your changes.
   • Your code has to be approved by a contributor familiar with the code.
   • All users are expected to take comments into account.
   • Comments tend to be written in an open and direct manner. The intent is to efficiently collaborate towards a solution we all agree on.
   • Once all discussions are resolved, a maintainer integrates the change.

For large PRs that are bound to evolve over time, keeping corrections during review in separate commits helps reviewers. In the case of multiple reviews by the same person, this can save full re-review (ask for help if necessary):

• Add fixup, amend, squash or reword commits using the git commit documentation.
• Automatically merge corrections into the original commits of your PR and check the result, using git rebase -i --autosquash origin/dev (just before the merge and once review process is complete).
• Push your changes with git push --force-with-lease because you are not just pushing new commits, you are pushing changes to existing commits.

3. If you believe somebody forgot to review / merge your change, please speak out, multiple times if needs be.

Suggested workflow

Here’s a suggested workflow.

It can be useful to communicate via instant messaging (Matrix, Slack, etc.) in order to guarantee the smooth flow of PR validation.

sequenceDiagram
  actor A as PR author
  actor R as Reviewer/Maintainer
  
  A->>R: Asks for a review, notifying some people
  R->>A: Answers yes or no
  
  loop Loop between author and reviewer
    R-->>A: Comments, asks for changes
    A-->>R: Answers to comments or requested changes
    A-->>R: Makes necessary changes in dedicated "fixups"
    R-->>A: Reviews, tests changes, and comments again
    R-->>A: Resolves requested changes/conversations if ok
  end
 
  A->>R: Rebase and apply fixups
  R->>A: Checks commits history
  R->>A: Approves or closes the PR
  Note left of R: & Merges if maintainer

Finally continue towards tests ‣