CodeReview

Requesting a Code Review

In order to request a code review you must create a pull request (PR) on GitHub.

Once you are happy that your PR is ready for review please give it the "ready for review" label and request a review from one or more people: Andy [andrewcoughtrie], Maff [mo-mglover] or Iva [TeranIvy]. Although you can request a review from multiple people, only one review actually needs to be performed.

A review is performed in GitHub by creating comments on points in the code that need attention. The PR author may respond to these comments and the result is a 'conversation'. It is up to the reviewer to decide when a conversation can be marked as 'resolved' (i.e. the point has been dealt with to their satisfaction).

Guidelines for Performing a Vernier Code Review

1. Check that branch is up-to-date with the main branch (the pull request should report that the branch can be "merged automatically"). If not, return to developer for them to bring it up-to-date (this can be done when requesting changes).
2. Check that there are no TODOs in the code that refer to the current issue/PR.
3. Use the "Files changed" tab on the pull request to review all code changes. Check that all code modifications are well commented, easy to understand and that the code is correct. Comments and requests for changes may be made in-line on the "Files changed" tab. This makes it easier for the developer to see which part of the code is being discussed.
4. If there are suggested or/and requested code changes then return the PR developer to address the review remarks.
5. Check that the copyright details are correct in any modified files.

Once the review is complete there are two options:

1. If the reviewer is happy with the pull request then they can proceed to merge the branch onto the main branch (see below).
2. If the reviewer is requesting changes, the request changes option in the review should be selected and it is then up to the original developer to address the reviewer's concerns.

Merging a Branch to Main

Once a pull request has passed code review, the code reviewer should merge the associated branch onto the main branch:

1. Check out the most recent version of the branch.
2. Check that all tests pass in this version (sanity check).
3. Commit these changes and push branch to GitHub (Commit message should include the pull request number).
4. On the pull-request page on GitHub, request the branch be merged to the main branch. Use Squash and merge option.
5. Check out the latest main branch and check that all tests still pass!
6. Delete the original branch.
7. If there is an Issue associated with the pull request, close it (note, this can be done automatically by linking the pull request to the associated issue).