Skip to content
Snippets Groups Projects

MergePolicy

Last edited by Alexander Færøy

Merge Policy

This policy was provisionally accepted by the network team on 14 October 2019. It became non-provisional on 14 January 2020.

For information on how to merge, see the org/teams/NetworkTeam/MergeProcess document.

SCOPE

This policy describes when we merge things to the main tor.git repository, and to other repositories owned by the network team.

It describes:

  • Who may merge.
  • Who should merge.
  • How many people must approve each merge.

THE GENERAL RULE

In general, every branch should be reviewed and approved by somebody other than the author before it is merged.

In other words, you can merge your own commits if somebody else has reviewed and approved them; you can merge other people's commits after you review them and approve them.

There are exceptions to this rule, listed below.

REQUESTING ADDITIONAL REVIEW

If the author or reviewer of a branch believes that it needs an additional reviewer, they should mark it as such, by saying so on the ticket, and by marking the ticket with "extra-review". (Additionally, telling people on IRC is also encouraged.)

This is especially recommended for:

  • tricky code
  • code without test coverage
  • security sensitive code
  • security fixes
  • changes to code that has caused bugs in the past
  • code that cannot be tested with CI
  • code that, for some reason, does not conform to our regular PR requirements, testing standards, or so on

Additional review is always required for any security patch that we intend to keep secret until release.

For nontrivial contributions from external volunteers, additional review is strongly recommended.

OLDER BRANCHES

Stable branches have backport maintainers. Only the designated backport maintainers for a particular branch should normally merge to it.

Generally, any patches that are merged into a stable branch should first be tested in an unstable branch. We may make an exception for urgent patches:

  • Medium or higher security issues, as defined in our security policy.

  • Major bugs that seriously threaten network stability.

  • Regressions that break compilation or CI on important platforms.

(Before you merge to any branch besides master, make sure you know how.)

Nobody should merge anything to an obsolete branch.

== WHEN IS REVIEW NOT NEEDED ==

The following items do not need a reviewer or a separate merger:

  • Putting out releases:

    • Creating the ChangeLog and ReleaseNotes
    • Bumping the version number
    • Creating a signed tag

  • Typo/grammar/spelling ("editorial") fixes in comments.

  • Typo/grammar/spelling ("editorial") fixes in documentation.

  • Whitespace fixes.

  • Running "make autostyle".

  • Editing a changes file.

WHEN NOT TO MERGE

For a day or two before a release, please do not merge anything at all into the release's branch without first checking with the release manager.

WHAT TO CHECK BEFORE MERGING

These issues are hard to notice (or hard to fix) after merging, so PLEASE be sure that you check them first.

 * Is there a changes file?

 * Is the patch based against the correct branch?

 * Is there a spec branch corresponding to any protocol change?

Also, PLEASE check to make sure that all pre-review requirements are really satisfied.

== OTHER NOTES ==

All mergers should make sure they are org/teams/NetworkTeam/MergeProcess.

The merger should, if possible, be on IRC when they're merging things.

Only merge when you are sober, awake, and alert.

If you feel funny about something, don't merge it, and tell the other team members.

Remember, above all, that Tor exists for its users, not for its developers: we all have the duty to protect user security and privacy. [This should be obvious, but it's useful to have a policy saying so.]

Comments

    Please register or sign in to add a comment.