move around some material on the reviewing doc page

This commit is contained in:
Paul Kehrer 2014-02-11 23:08:47 -06:00
parent 18962cc15b
commit 91c776fe1d
2 changed files with 26 additions and 20 deletions

View file

@ -1,26 +1,8 @@
Reviewing/Merging Patches
=========================
Because cryptography is so complex, and the implications of getting it wrong so
devastating, ``cryptography`` has a strict code review policy:
* Patches must *never* be pushed directly to ``master``, all changes (even the
most trivial typo fixes!) must be submitted as a pull request.
* A committer may *never* merge their own pull request, a second party must
merge their changes. If multiple people work on a pull request, it must be
merged by someone who did not work on it.
* A patch that breaks tests, or introduces regressions by changing or removing
existing tests should not be merged. Tests must always be passing on
``master``.
* If somehow the tests get into a failing state on ``master`` (such as by a
backwards incompatible release of a dependency) no pull requests may be
merged until this is rectified.
* All merged patches must have 100% test coverage.
The purpose of these policies is to minimize the chances we merge a change
that jeopardizes our users' security.
When reviewing a patch try to keep each of these concepts in mind:
Everyone is encouraged to review open pull requests. When reviewing a patch try
to keep each of these concepts in mind:
Architecture
------------
@ -49,3 +31,26 @@ These are small things that are not caught by the automated style checkers.
* Does a variable need a better name?
* Should this be a keyword argument?
Merge Requirements
------------------
Because cryptography is so complex, and the implications of getting it wrong so
devastating, ``cryptography`` has a strict merge policy for committers:
* Patches must *never* be pushed directly to ``master``, all changes (even the
most trivial typo fixes!) must be submitted as a pull request.
* A committer may *never* merge their own pull request, a second party must
merge their changes. If multiple people work on a pull request, it must be
merged by someone who did not work on it.
* A patch that breaks tests, or introduces regressions by changing or removing
existing tests should not be merged. Tests must always be passing on
``master``.
* If somehow the tests get into a failing state on ``master`` (such as by a
backwards incompatible release of a dependency) no pull requests may be
merged until this is rectified.
* All merged patches must have 100% test coverage.
The purpose of these policies is to minimize the chances we merge a change
that jeopardizes our users' security.

View file

@ -3,6 +3,7 @@ backends
boolean
ciphertext
committer
committers
crypto
cryptographic
cryptographically