|
|
# 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 [MergeProcess](./MergeProcess) document.
|
|
|
For information on **how** to merge, see the [MergeProcess](./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.
|
|
|
|
|
|
This policy describes when we merge things to the main tor.git repository, and
|
|
|
to other repositories owned by the network team.
|
|
|
|
|
|
## THE GENERAL RULE
|
|
|
|
|
|
In general, every branch should be reviewed and approved by somebody other than the author before it is merged.
|
|
|
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.
|
|
|
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.)
|
|
|
If the author or reviewer of a branch believes that it needs an additional
|
|
|
reviewer, they should ask for such to a specific reviewer in the ticket.
|
|
|
|
|
|
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.
|
|
|
* 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
|
|
|
|
|
|
## OLDER BRANCHES
|
|
|
Additional review is always required for any security patch that we intend to
|
|
|
keep secret until release.
|
|
|
|
|
|
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.
|
|
|
For nontrivial contributions from external volunteers, additional review is
|
|
|
strongly recommended.
|
|
|
|
|
|
## 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
|
|
|
* Putting out releases: Creating the ChangeLog and ReleaseNotes Bumping the
|
|
|
* version number Creating a signed tag
|
|
|
|
|
|
* Typo/grammar/spelling ("editorial") fixes in comments.
|
|
|
|
... | ... | @@ -80,11 +55,13 @@ The following items do not need a reviewer or a separate merger: |
|
|
|
|
|
## 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.
|
|
|
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.
|
|
|
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?
|
|
|
|
... | ... | @@ -92,16 +69,16 @@ These issues are hard to notice (or hard to fix) after merging, so PLEASE be sur |
|
|
|
|
|
* Is there a spec branch corresponding to any protocol change?
|
|
|
|
|
|
Also, PLEASE check to make sure that all [pre-review requirements](https://gitweb.torproject.org/tor.git/tree/doc/HACKING/Maintaining.md#n80) are really satisfied.
|
|
|
|
|
|
## OTHER NOTES
|
|
|
|
|
|
All mergers should make sure they are [using our scripts and git hooks](./MergeProcess).
|
|
|
|
|
|
The merger should, if possible, be on IRC when they're merging things.
|
|
|
All mergers should make sure they are [using our scripts and git
|
|
|
hooks](./MergeProcess).
|
|
|
|
|
|
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.
|
|
|
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.] |
|
|
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.] |