Reviewing Pull Requests

Review is an important process where you should ensure requirements are clear, all functionality is properly implemented, tests are present and code quality satisfies Status and your own quality bar.

Goal

The goal of the reviewer is to approve the pull request.

Finding all styling, minor and naming bugs is not a goal. Thus, a reviewer should request changes only if one of the steps described below fails or if there's a major architectural decision which one considers controversial.

If there's bad (but not critical) responsibilities separation, if you don't like naming or the structure of code, you should not request changes. Make a note for yourself and include it to a refactoring task to fix that, we always include a number of refactoring tasks in sprints.

Steps

1. If there's a connected issue, read its description and comprehend the requirements. If you don't understand the requirements, further specify them with the implementor so that they're clear for other people.
2. Ensure tests pass.
3. Examine the source code and ensure all the requested functionality is fully and properly implemented. If not, either fix the requirements or ask to finish the task. Also, build the project and try out new features yourself.
4. Ensure tests for the new code are written.
5. Ensure the code is compliant with Status Code Quality Notes and your quality bar.
6. If everything is alright, approve the pull request. If not, decline and move it to the Assigned pipeline.
7. If you're responsible for merging, check out the Future Steps in the associated issue and ask the implementor to follow these steps.

All steps are required but you can skip any of them in case you have reasons to do so.

Respect the other person's code even if it seems not so good from the first glance. Often you may not know everything about the code what the implementor knows so it's better to refrain from categorical statements and ask questions instead.