Reviewing and merging patches 

Everyone is encouraged to review open pull requests. We only ask that you try and think carefully, ask questions and are excellent to one another . Code review is our opportunity to share knowledge, design ideas and make friends.

When reviewing a patch try to keep each of these concepts in mind:

Intent 

• What is the change being proposed?

• Do we want this feature or is the bug they’re fixing really a bug?

Architecture 

• Is the proposed change being made in the correct place? Is it a fix in the backend when it should be in the primitives?

Implementation 

• Does the change do what the author claims?

• Are there sufficient tests?

• Has it been documented?

• Will this change introduce new bugs?

Grammar and style 

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 main , 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 main .

• If somehow the tests get into a failing state on main (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.