diff options
| author | Paul Kehrer <paul.l.kehrer@gmail.com> | 2014-02-11 23:08:47 -0600 |
|---|---|---|
| committer | Paul Kehrer <paul.l.kehrer@gmail.com> | 2014-02-11 23:08:47 -0600 |
| commit | 91c776fe1d577efa67beb4ea26caf9492bae4070 (patch) | |
| tree | 36cfad11882104f15245abea8eebb0524796f802 /docs/development/reviewing-patches.rst | |
| parent | 18962cc15b8562e46ea8b6f60fb5072c344d9b12 (diff) | |
| download | cryptography-91c776fe1d577efa67beb4ea26caf9492bae4070.tar.gz cryptography-91c776fe1d577efa67beb4ea26caf9492bae4070.tar.bz2 cryptography-91c776fe1d577efa67beb4ea26caf9492bae4070.zip | |
move around some material on the reviewing doc page
Diffstat (limited to 'docs/development/reviewing-patches.rst')
| -rw-r--r-- | docs/development/reviewing-patches.rst | 45 |
1 files changed, 25 insertions, 20 deletions
diff --git a/docs/development/reviewing-patches.rst b/docs/development/reviewing-patches.rst index 13d9cd8f..302c998e 100644 --- a/docs/development/reviewing-patches.rst +++ b/docs/development/reviewing-patches.rst @@ -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. + |