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 they do, or if in doubt, create an issue to ask our technical writers to update the documentation accordingly.

We generally prepare documentation updates in the same branch as the corresponding code changes, so that both get released together.

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

Do not break the build... or get reverted

Do not push changes breaking the build into one of our main Git branches: master, stable, testing, and devel.

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.

Coordinate major changes and freeze exceptions with Release Managers

See the coordination guidelines for developers with Release Managers.

How to submit your proposed changes

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

All proposed changes should be prepared in topic branches. A branch about issue number NNNN should be named NNNN-XXX.

To submit your branch for review:

Test your branch. If your branch contains code changes, test it with its target branch (config/base_branch) merged into it.
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 own fork of the Tails Git repository.
Once you would like your branch to be reviewed, and possibly merged, submit it:
1. If your branch contains code changes and you have access to our Jenkins instance:
  - Ensure your branch builds on Jenkins.
  - Either take note of the test suite scenarios you've seen pass successfully locally, or check that the test suite passes on Jenkins.
2. On the corresponding GitLab issue:
  - Set the Milestone field to the release you would like your changes to land in.
  - If there's a To Do or Doing label, replace it with the Needs Validation label.
3. Create a Merge Request (aka. MR). There are many ways to do so. For example, you can click the Create merge request button on the corresponding issue.
  
  In this new MR:
  - Use the description to:
    - Summarize what problem this MR will fix, in terms of impact on users.
    - Reference the issues this MR will solve: Closes #xxx, #yyyy.
  - Set the target branch your branch should be merged into. See the documentation for our main branches.
  - If it's obvious to you who can, and should, review your branch, and you're allowed to request reviews on MRs: request a review by making that person the Reviewer on the MR.
    Else, leave the MR with no Reviewer: the Foundations Team will either handle the review themselves or help you find a suitable reviewer.
  - If you have run some test suite scenarios locally: report about your results in a new comment.

Then, for every subsequent submit/review/fix cycle: once you've fixed all problems raised by the reviewer, update the issue and MR metadata again as documented above (milestone, reviewer, and labels).

How to review and merge proposed changes

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:

Give feedback about these problems.
Unset the Reviewer on the MR.
On the corresponding issue: replace the Needs Validation label with Doing.

Merge

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