We use GitLab for tracking issues, merge requests, and code. Please ensure you are familiar with how we use GitLab at Tails.

In particular, see how to participate in discussions.

Review

• Set yourself as the Reviewer on the Merge Request to you. This avoids duplicating work.

• Verify that the destination branch matches what's in config/base_branch on the branch, and is correct:

  • for a new feature: into devel

  • for a fix on top on the last RC: into testing; then merge testing into devel

  • for a fix on top of the last stable: into stable; then merge stable into devel

• Merge the destination branch of the MR into the topic branch.

• Build the topic branch and test the feature.

• Check automated ISO build and test results on https://jenkins.tails.boum.org/.

• Review the diff, either in the GitLab interface or on the command line. If you're using the command line, beware of changes introduced by merge commits: either add the --cc option to git log, or use git diff after reviewing the individual patches.

• Check the contents of every APT overlay that the branch enables:

  • If you have upload rights to our custom APT repository:

    ssh reprepro@incoming.deb.tails.boum.org \
        reprepro list ${APT_overlay}`
    

  • Else:

    • https://deb.tails.boum.org/dists/${APT_overlay}/main/source/Sources
    • https://deb.tails.boum.org/dists/${APT_overlay}/main/binary-amd64/Packages

• Check the user and design documentation.

• Check the corresponding GitLab issue.

• Ensure the description of the MR includes Closes #NNNN statements for the issues fixed by this MR.

• Changes proposed by new contributors, or by the patch'n'forget kind, shall respectively be applied once they are in good enough state. That is, once the core developers team feels like maintaining it on the long run in case the initial submitter disappears. Such maintenance includes: polishing the proposed changes to make them fit for release; writing and updating the design and end-user documentations; fixing bugs.

• As reviewer, you are allowed to commit trivial fixes on top of the proposed branch to avoid round-trips: for example, fixing typos and improving phrasing of comments and strings. Then, report back about these changes on the MR.

Give feedback

First, remember that it's hard to receive negative feedback. Don't forget to note the good parts, be constructive and precise in your comments, and never use reviews to make personal attacks. You can read these blog posts on review and feedback:

• Kate Heddleston: Criticism and ineffective feedback (blog post on how to do a good review)
• Liane Davey: Maybe you shouldn't give feedback

As part of your review, if you spot problems that block the merge in your opinion:

1. Give feedback about these problems.

2. Unset the Reviewer on the MR.

3. On the corresponding issue: replace the Needs Validation label with Doing.

Merge

1. Click the Merge button on the MR.

   If you are not authorized to do so, instead:

   • Add a comment on the MR to say you have reviewed the branch. If you approve its merging, click the Approve button.
   • Unset the Reviewer on the MR.

2. Close the issues fixed by this MR.

   You don't need to do this if the destination branch of the MR is stable: these issues will be closed automatically.

3. If the destination branch of the MR was stable, then pull stable locally, merge it into devel, and push devel.

   Rationale: devel must also contain all the improvements that have been applied on stable.