Skip to content

Guideline: Reviewing a pull request

Agah edited this page Dec 20, 2018 · 5 revisions

Reviewing a pull request

Below are a list of recommended guidelines of the steps that you should take if someone has recommended you as a reviewer for a pull request. Because qMRLab's graphical user interface (GUI) cannot be tested using the continuous integration service Travis, it's strongly recommended you do the manual GUI tests listed at the end, unless you are absolutely certain that the changes in the commit cannot impact the user experience through the GUI in any way (e.g. simple documentation changes).

For step 7 ("Test new features or bug fix"), a good practice would be to set break points at the beginning of each section of the code that was changed, and walk through each line using MATLAB's debugger to ensure the behaviour is as intended.

Recommended guidelines

1. Ensure that branch is up-to-date with master

  • If not, merge master into the branch
  • Carefully identify and review any changes in compiled/binary files – particularly important for .fig files. If the master included any changes in these after the PR was branched, then you'll have to identify and manually merge the new elements of the PR into the master one (NOT chose one file over the other).

2. Travis tests pass?

  • If not, resolves bugs/issues that caused the fail.

3. (Optional) Run all MOxUnit tests in MATLAB

4. Coveralls.io condition pass?

  • Fails if coverage drops by more than 1%
  • If it fails, write more tests that cover the new code from the branch.

5. Review that changed source files/lines are related to the pull request/issue

  • If any files/commits were accidentally included, cherry-pick them into another branch.

6. Review that changed source files/lines were not accidentally deleted

  • Fix appropriately if so.

7. Test new features or bug fix

  • If not implemented/resolved adequately, solve it or inform the developer by requesting changes in your review.
  • Preferably, set breakpoints in the locations that the code was changed and follow allong line by line to see if the code behaves as intended.

8. Manual GUI tests (general)

  • Does the qMRLab GUI open?
  • Can you change models?
  • Can you load a data folder for a model?
  • Can you view data?
  • Can you zoom in the image?
  • Can you pan out of the image?
  • Can you view the histogram of the data?
  • Can you change the color map?
  • Can you fit dataset (Fit data)?
  • Can you save/load the results?
  • Can you open the options panel?
  • Can you change option parameters?
  • Can you save/load option paramters?
  • Can you select a voxel?
  • Can you fit the data of that voxel ("View data fit")?
  • Can you simulate and fit a voxel ("Single Voxel Curve")?
  • Can you run a Sensitivity Analysis?
  • Can you simulate a Multi Voxel Distribution?