Review process
How to give useful feedback
The reviewer/maintainer undertakes to carry out the review quickly, and is also responsible for closing request changes, check commit history and quickly merge the pull request if allowed.
We propose you a few tips and recommendations that we think are relevant to a human, relevant and rewarding code review for all contributors:
- How to Make Your Code Reviewer Fall in Love with You? by Michael Lynch.
- How to Do Code Reviews Like a Human? by Michael Lynch.
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
If the reviewer is not maintainer, the PR’s author has the responsability to contact a maintainer to merge the PR. In special cases (especially near feature freeze), maintainer should be found early.