Code Review

Code review is finally fun and fast thanks to MergeBoard. Here we will tackle, how the reviewer and author interact and what makes this process so much safer and easier to track.

Review Loop

You have already been introduced to actors and how MergeBoard calculates who is currently responsible for the advancement of the merge request. MergeBoard makes it very easy for you to track this responsibility if you follow the review loop:

1 Author opens the merge request or pushes changes

The first thing MergeBoard will do is wait. While the CI and Scanners are running the responsibility for advancing the merge request lies with the CI and Scanners.

  • If the author fixed change requests, they now mark them as Fixed or Intentional to let the reviewer(s) know these tasks are solved. While there are open change requests, the responsibility for the merge request stays with the author.

2 The CI results are in:

  • If the CI or the Scanners report a failed status, the author gets the responsibility for fixing these errors. Once they push their changes, we return to step 1.

  • If the results are good, the reviewer(s) get the responsibility for advancing the merge request.

3 Review

While reviewing each reviewer can add comments and change requests, preferably by adding all review comments and requests and then sending all pending comments at once.

They can also check off all change request they deem correctly implemented or remove the Fixed or Intentional flag set by the author to let them know, these tasks need to be tackled again. This is also a good time to add a comment, why they think this is.

  • If there are open change requests the responsibility now is handed back to the author again. We are back at step 1.

  • If all change requests implementations are approved the merge request is now ready to merge and can leave the loop.

4 Merge

Anyone with the necessary rights can finally merge the merge request. MergeBoard will now trigger certain actions depending on you project settings and e.g. close any connected issues.

Please note that this way of working is not enforced by MergeBoard. Both reviewers and author can e.g. post change requests at any given time or push changes to the merge request. Nevertheless, this is the intended way of using MergeBoard, though if you are having other workflows that you want to solve with our tools, let us know at support@mergeboard.com.

Communication

Communication is key, this is why we give you the possibility to be very precise in what you say, request and consider done.

MergeBoard allows you to communicate in threads, meaning that every user can post some comment or change request which should then be discussed in its attached thread. Scanner results can also be discussed in this form.

Commenting

There are two ways to add a comment in a merge request.

  1. You can use the comment box in the merge request overview to leave a general comment on the merge request.

  2. You can comment on specific code in the merge request’s commit or changes view.

    ../../_images/comments-small.gif

    Drag your mouse cursor (left click + move) in the area between the line numbers and code to reference the corresponding lines with your comment. You can repeat this process to reference multiple independent code areas (even across files) from a single comment. If you want to deselect a line again, simply use your right mouse button and drag across the selected lines.

    When you are satisfied with your selection, click the plus button to open the comment editor. The plus button is located at the last marked position.

../../_images/thread_comment_edit.png

As comment author you can always edit your comments later on by clicking the edit button, shown on the right. This will open an editor in which you can manipulate your comment.

Threads can be collapsed by clicking the initial comment’s header bar.

../../_images/thread_add_hide.png

While being in a diff view you can hide threads entirely by clicking the small thread collapse icon on the right side of the line, the comment has been made on.

Threads can be unhidden by hovering their corresponding icon next to the line they are referencing and clicking that icon.

Comment Threads

../../_images/comment_toggle.png

Used for general discussions or feedback. Make sure to select the green speech bubble when writing your comment.

The intended use for these threads is to be a place of general discussion, praising each other’s good work or anything that does not block the merge request from advancing without being explicitly addressed via changes to the code.

Change Requests

../../_images/change_request_toggle.png

Used to communicate specific change requests. Works best in combination with a code reference. Make sure to select the red bug when writing your request.

While change requests have all the features of a normal comment thread they also track their implementation state. This is done using the green buttons on the top right of the thread comment.

open

After the change request is posted its state is open, indicated by the fact that it is neither marked as Fixed nor Intentional.

fixed intentional

While open you can change the state to Fixed by simply clicking on the button or by selecting the state from the dropdown. Here you can also select the state Intentional. Now the change request is marked as addressed.

f_approved i_approved

After having set the state of a change request to addressed, a green checkmark appears. A reviewer can click it to mark the thread as approved. Alternatively a reviewer or the author can set the thread’s state back to open by clicking on Fixed or Intentional, depending on how the thread was marked as addressed.

Comments and Revisions

thread_rev_ctrl

If a thread, be it plain comment or change request, references code, this code is displayed alongside the comment in the merge request overview. As the referenced code might change across revisions, MergeBoard offers you the possibility to switch between revisions directly inside the comment. Use the arrows to step through revisions or the dropdown menu to directly select the revision you want to look at. The revision the comment was made during is highlighted with an exclamation marker.

Keeping Track

When a merge request is large or consists of many revisions, it can sometimes be difficult to keep track of what you have already reviewed. To make this easier, you can mark in MergeBoard which lines you have reviewed. These markers are only visible to you.

../../_images/reviewed-small.gif

Drag your mouse cursor (left click + move) over the line numbers in the merge request’s commit or changes view to mark a line as reviewed. To remove your review mark, simply drag across the corresponding lines while pressing your right mouse button.

MergeBoard propagates the review markers across commits and revisions. For example, when reviewing the changes of a merge request, you can either go through the changes commit by commit or view all changes at once in the changes view. When you mark a line as reviewed in a commit, the same line will be shown as reviewed in the changes view, unless the line was modified or deleted by a later commit.

The review marker is removed again if the line is modified by a new revision, so you always know which code areas still need to be reviewed.