1Reviewing and merging patches
2=============================
3
4Everyone is encouraged to review open pull requests. We only ask that you try
5and think carefully, ask questions and are `excellent to one another`_. Code
6review is our opportunity to share knowledge, design ideas and make friends.
7
8When reviewing a patch try to keep each of these concepts in mind:
9
10Architecture
11------------
12
13* Is the proposed change being made in the correct place? Is it a fix in a
14  backend when it should be in the primitives?
15
16Intent
17------
18
19* What is the change being proposed?
20* Do we want this feature or is the bug they're fixing really a bug?
21
22Implementation
23--------------
24
25* Does the change do what the author claims?
26* Are there sufficient tests?
27* Has it been documented?
28* Will this change introduce new bugs?
29
30Grammar and style
31-----------------
32
33These are small things that are not caught by the automated style checkers.
34
35* Does a variable need a better name?
36* Should this be a keyword argument?
37
38Merge requirements
39------------------
40
41Because cryptography is so complex, and the implications of getting it wrong so
42devastating, ``cryptography`` has a strict merge policy for committers:
43
44* Patches must *never* be pushed directly to ``master``, all changes (even the
45  most trivial typo fixes!) must be submitted as a pull request.
46* A committer may *never* merge their own pull request, a second party must
47  merge their changes. If multiple people work on a pull request, it must be
48  merged by someone who did not work on it.
49* A patch that breaks tests, or introduces regressions by changing or removing
50  existing tests should not be merged. Tests must always be passing on
51  ``master``.
52* If somehow the tests get into a failing state on ``master`` (such as by a
53  backwards incompatible release of a dependency) no pull requests may be
54  merged until this is rectified.
55* All merged patches must have 100% test coverage.
56
57The purpose of these policies is to minimize the chances we merge a change
58that jeopardizes our users' security.
59
60.. _`excellent to one another`: https://speakerdeck.com/ohrite/better-code-review
61