Common rules

Documentation is not optional

When writing Tails code, one commits to adapt the design and end-user documentations accordingly in a timely manner, writing brand new chapters if needed.

Design documentation

As a side note, best is generally to write specification and design documentation before implementing changes; among other very valid reasons to do so, it may avoid doing work that won't be applied ever, or be reverted later, because of a faulty design that was reviewed and diagnosed only when the implementation was up and running. On the other hand, we're not great fans of over-engineering and we do know proceeding like this is not always an option, as the right design sometimes arises from erratic implementation attempts.

User documentation

See our documentation guidelines.

Verify that your changes do not affect the rest of the user documentation and FAQ. If so, fix it in your development branch so that it gets automatically integrated when your branch get released.

Regarding the FAQ, don't write new questions in advance but make sure that the existing ones are still correct.

Test your code

A branch proposed for review and merge must have been tested when applied against the target branch(es) of the requested merge.

Do not break the build... or get reverted

Noone should ever push changes breaking the build into the shared Git repository. On the other hand, this may happen since preliminary, untested changes may in some circumstances land into our devel branch to be reviewed and get more exposure.

If you find the devel branch in a non-building state and can identify the root cause of it to a recent commit, fix it if you wish, but don't let it disturb you otherwise: just revert the faulty commit and inform the other developers on tails-dev@boum.org so that the author knows s/he needs to fix his/her stuff before reapplying it at a later point.

How to submit your changes

All development should happen in topic branches. For a new feature XXX, it should be named feature/XXX. For a bugfix about YYY, it should be named bugfix/YYY. Ideally, include the relevant ticket number in the topic branch name, e.g. bugfix/7173-upgrade-syslinux.

When you think it is good enough and have tested it, you have to:

  1. Push your branch
    • If you have commit access to the official Tails Git repository, push your branch there so our CI picks it up.
    • Else, push to your personal Git repository: fork us on Salsa.
  2. Set the ticket's Status field to In Progress (if you do not see this field when editing the ticket, ask the sysadmin team to grant you the necessary permissions).
  3. Point the ticket's Feature Branch field either to your branch or to a merge request on Salsa.
  4. Set the ticket's Target version field to the release you would like your changes to be in.
  5. Make it clear what you're requesting: merging? some advice? an initial code review of work that's not finished yet?
  6. If you have access to our Jenkins instance and you are requesting a merge:
    • Ensure your branch builds on Jenkins.
    • Either report about the test suite scenarios you've seen pass successfully locally, or check that the test suite passes on Jenkins.
  7. Set the ticket's Status field to Needs Validation.
  8. Set the ticket's Assignee field appropriately:
    • If it's already obvious to you who can and should review your branch: assign the ticket to this person.
    • Else, assign the ticket to nobody, i.e. unassign it from yourself. The Foundations Team will either handle the review themselves or help you find a suitable reviewer.
  9. For important changes, if you feel the need to ask input from the greater development community, notify the tails-dev@boum.org mailing list.

Then, if the reviewer asks for more development to be done before merging, they should set the ticket's Status field back to In Progress; from now on it's the responsibility of the branch/ticket "holder" to change it back to Needs Validation once they consider the issues raised by the reviewer are fixed.

The reviewer is 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. They must report back about these changes on the ticket.

Review and merge process

Review

  • When you start doing a review'n'merge, assign the relevant ticket and merge request 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 contents of every APT overlay that the branch enables:

  • 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.
  • 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 ticket.
  • 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

If the proposed change was submitted as a merge request on Salsa, don't click Merge there: while we can use GitLab for reviews, our infrastructure is not ready to consume merge operations done there, so you need to merge on the command line.

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 Closes: #NNNN in the commit message, NNNN being the ticket number that is fixed by the branch you are merging. Then, Redmine will automatically close the corresponding ticket once you push the results of your merge to our main Git repository. For example:

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

Book keeping

  1. Update the Status field on the ticket. If there is no remaining tasks listed on the ticket, then change its status to Resolved (unless you used the Closes keyword documented above); else, set it back to In Progress and 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.