Review

  • When you start doing a review'n'merge, assign the relevant ticket to you, in order to avoid duplicated work.
  • Merge the base branch (the one you're supposed to merge the reviewed topic branch into, as specified in config/base_branch and in the pull request -- they must match) 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/.
  • Check the diff e.g. with git log -p. 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 APT suite with ssh reprepro@incoming.deb.tails.boum.org reprepro list [bugfix|feature]-<name-with-dashes>
  • Check the user and design documentation.
  • Check the ticket.
  • 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.
  • 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:

Merge

Merge the branch with --no-ff:

  • 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

Please consider including fix-committed: #NNNN in the commit message, NNNN being the ticket number that is fixed by the branch you are merging. Then, Redmine will automatically flag the corresponding ticket as "Fix committed" once you push the results of your merge to our main Git repository. For example:

Merge branch 'bugfix/8470-clean-up-apt-pinning' (Fix-committed: #8470)

Book keeping

  1. Update the QA Check field on the ticket. If there is no remaining tasks listed on the ticket, then change its status to Fix committed (unless you used the fix-committed keyword documented above); else, ask the branch submitter to split the remaining tasks into other tickets.
  2. Push the updated branch to the master Git repository.
  3. Reply to the email that requested the review, if any.