From 0839aa858f41f71b292c387f6bc3641083b965bc Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Tue, 11 Feb 2014 22:36:51 -0600 Subject: reorganize contributing into development section --- docs/contributing.rst | 299 -------------------------------- docs/development/getting-started.rst | 84 +++++++++ docs/development/index.rst | 17 ++ docs/development/reviewing-patches.rst | 51 ++++++ docs/development/submitting-patches.rst | 199 +++++++++++++++++++++ docs/index.rst | 2 +- 6 files changed, 352 insertions(+), 300 deletions(-) delete mode 100644 docs/contributing.rst create mode 100644 docs/development/getting-started.rst create mode 100644 docs/development/index.rst create mode 100644 docs/development/reviewing-patches.rst create mode 100644 docs/development/submitting-patches.rst diff --git a/docs/contributing.rst b/docs/contributing.rst deleted file mode 100644 index 87075638..00000000 --- a/docs/contributing.rst +++ /dev/null @@ -1,299 +0,0 @@ -Contributing -============ - -Process -------- - -As an open source project, ``cryptography`` welcomes contributions of all -forms. These can include: - -* Bug reports and feature requests -* Pull requests for both code and documentation -* Patch reviews - -You can file bugs and submit pull requests on `GitHub`_. To discuss larger -changes you can start a conversation on `our mailing list`_. - -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. -* New features and significant bug fixes should be documented in the - :doc:`/changelog`. - -The purpose of these policies is to minimize the chances we merge a change -that jeopardizes our users' security. - -If you believe you've identified a security issue in ``cryptography``, please -follow the directions on the :doc:`security page `. - -Code ----- - -When in doubt, refer to :pep:`8` for Python code. - -`Write comments as complete sentences.`_ - -Every code file must start with the boilerplate notice of the Apache License. -Additionally, every Python code file must contain - -.. code-block:: python - - from __future__ import absolute_import, division, print_function - -API Considerations -~~~~~~~~~~~~~~~~~~ - -Most projects' APIs are designed with a philosophy of "make easy things easy, -and make hard things possible". One of the perils of writing cryptographic code -is that secure code looks just like insecure code, and its results are almost -always indistinguishable. As a result ``cryptography`` has, as a design -philosophy: "make it hard to do insecure things". Here are a few strategies for -API design that should be both followed, and should inspire other API choices: - -If it is necessary to compare a user provided value with a computed value (for -example, verifying a signature), there should be an API provided that performs -the verification in a secure way (for example, using a constant time -comparison), rather than requiring the user to perform the comparison -themselves. - -If it is incorrect to ignore the result of a method, it should raise an -exception, and not return a boolean ``True``/``False`` flag. For example, a -method to verify a signature should raise ``InvalidSignature``, and not return -whether the signature was valid. - -.. code-block:: python - - # This is bad. - def verify(sig): - # ... - return is_valid - - # Good! - def verify(sig): - # ... - if not is_valid: - raise InvalidSignature - -Every recipe should include a version or algorithmic marker of some sort in its -output in order to allow transparent upgrading of the algorithms in use, as -the algorithms or parameters needed to achieve a given security margin evolve. - -APIs at the :doc:`/hazmat/primitives/index` layer should always take an -explicit backend, APIs at the recipes layer should automatically use the -:func:`~cryptography.hazmat.backends.default_backend`, but optionally allow -specifying a different backend. - -C bindings -~~~~~~~~~~ - -When binding C code with ``cffi`` we have our own style guide, it's pretty -simple. - -Don't name parameters: - -.. code-block:: c - - // Good - long f(long); - // Bad - long f(long x); - -...unless they're inside a struct: - -.. code-block:: c - - struct my_struct { - char *name; - int number; - ...; - }; - -Include ``void`` if the function takes no arguments: - -.. code-block:: c - - // Good - long f(void); - // Bad - long f(); - -Wrap lines at 80 characters like so: - -.. code-block:: c - - // Pretend this went to 80 characters - long f(long, long, - int *) - -Include a space after commas between parameters: - -.. code-block:: c - - // Good - long f(int, char *) - // Bad - long f(int,char *) - -Values set by ``#define`` should be assigned the appropriate type. If you see -this: - -.. code-block:: c - - #define SOME_INTEGER_LITERAL 0x0; - #define SOME_UNSIGNED_INTEGER_LITERAL 0x0001U; - #define SOME_STRING_LITERAL "hello"; - -...it should be added to the bindings like so: - -.. code-block:: c - - static const int SOME_INTEGER_LITERAL; - static const unsigned int SOME_UNSIGNED_INTEGER_LITERAL; - static const char *const SOME_STRING_LITERAL; - -Documentation -------------- - -All features should be documented with prose. - -Docstrings should be written like this: - -.. code-block:: python - - def some_function(some_arg): - """ - Does some things. - - :param some_arg: Some argument. - """ - -So, specifically: - -* Always use three double quotes. -* Put the three double quotes on their own line. -* No blank line at the end. -* Use Sphinx parameter/attribute documentation `syntax`_. - -Because of the inherent challenges in implementing correct cryptographic -systems, we want to make our documentation point people in the right directions -as much as possible. To that end: - -* When documenting a generic interface, use a strong algorithm in examples. - (e.g. when showing a hashing example, don't use - :class:`~cryptography.hazmat.primitives.hashes.MD5`) -* When giving prescriptive advice, always provide references and supporting - material. -* When there is real disagreement between cryptographic experts, represent both - sides of the argument and describe the trade-offs clearly. - -When documenting a new module in the ``hazmat`` package, its documentation -should begin with the "Hazardous Materials" warning: - -.. code-block:: rest - - .. hazmat:: - -When referring to a hypothetical individual (such as "a person receiving an -encrypted message") use gender neutral pronouns (they/them/their). - -Development Environment ------------------------ - -Working on ``cryptography`` requires the installation of a small number of -development dependencies. These are listed in ``dev-requirements.txt`` and they -can be installed in a `virtualenv`_ using `pip`_. Once you've installed the -dependencies, install ``cryptography`` in ``editable`` mode. For example: - -.. code-block:: console - - $ # Create a virtualenv and activate it - $ pip install --requirement dev-requirements.txt - $ pip install --editable . - -You are now ready to run the tests and build the documentation. - -Running Tests -~~~~~~~~~~~~~ - -``cryptography`` unit tests are found in the ``tests/`` directory and are -designed to be run using `pytest`_. `pytest`_ will discover the tests -automatically, so all you have to do is: - -.. code-block:: console - - $ py.test - ... - 62746 passed in 220.43 seconds - -This runs the tests with the default Python interpreter. - -You can also verify that the tests pass on other supported Python interpreters. -For this we use `tox`_, which will automatically create a `virtualenv`_ for -each supported Python version and run the tests. For example: - -.. code-block:: console - - $ tox - ... - ERROR: py26: InterpreterNotFound: python2.6 - py27: commands succeeded - ERROR: pypy: InterpreterNotFound: pypy - ERROR: py32: InterpreterNotFound: python3.2 - py33: commands succeeded - docs: commands succeeded - pep8: commands succeeded - -You may not have all the required Python versions installed, in which case you -will see one or more ``InterpreterNotFound`` errors. - - -Explicit Backend Selection -~~~~~~~~~~~~~~~~~~~~~~~~~~ - -While testing you may want to run tests against a subset of the backends that -cryptography supports. Explicit backend selection can be done via the -``--backend`` flag. This flag should be passed to ``py.test`` with a comma -delimited list of backend names. To use it with ``tox`` you must pass it as -``tox -- --backend=openssl``. - -Building Documentation -~~~~~~~~~~~~~~~~~~~~~~ - -``cryptography`` documentation is stored in the ``docs/`` directory. It is -written in `reStructured Text`_ and rendered using `Sphinx`_. - -Use `tox`_ to build the documentation. For example: - -.. code-block:: console - - $ tox -e docs - ... - docs: commands succeeded - congratulations :) - -The HTML documentation index can now be found at -``docs/_build/html/index.html``. - - -.. _`GitHub`: https://github.com/pyca/cryptography -.. _`our mailing list`: https://mail.python.org/mailman/listinfo/cryptography-dev -.. _`Write comments as complete sentences.`: http://nedbatchelder.com/blog/201401/comments_should_be_sentences.html -.. _`syntax`: http://sphinx-doc.org/domains.html#info-field-lists -.. _`pytest`: https://pypi.python.org/pypi/pytest -.. _`tox`: https://pypi.python.org/pypi/tox -.. _`virtualenv`: https://pypi.python.org/pypi/virtualenv -.. _`pip`: https://pypi.python.org/pypi/pip -.. _`sphinx`: https://pypi.python.org/pypi/Sphinx -.. _`reStructured Text`: http://sphinx-doc.org/rest.html diff --git a/docs/development/getting-started.rst b/docs/development/getting-started.rst new file mode 100644 index 00000000..b367119e --- /dev/null +++ b/docs/development/getting-started.rst @@ -0,0 +1,84 @@ +Getting Started +=============== + +Working on ``cryptography`` requires the installation of a small number of +development dependencies. These are listed in ``dev-requirements.txt`` and they +can be installed in a `virtualenv`_ using `pip`_. Once you've installed the +dependencies, install ``cryptography`` in ``editable`` mode. For example: + +.. code-block:: console + + $ # Create a virtualenv and activate it + $ pip install --requirement dev-requirements.txt + $ pip install --editable . + +You are now ready to run the tests and build the documentation. + +Running Tests +~~~~~~~~~~~~~ + +``cryptography`` unit tests are found in the ``tests/`` directory and are +designed to be run using `pytest`_. `pytest`_ will discover the tests +automatically, so all you have to do is: + +.. code-block:: console + + $ py.test + ... + 62746 passed in 220.43 seconds + +This runs the tests with the default Python interpreter. + +You can also verify that the tests pass on other supported Python interpreters. +For this we use `tox`_, which will automatically create a `virtualenv`_ for +each supported Python version and run the tests. For example: + +.. code-block:: console + + $ tox + ... + ERROR: py26: InterpreterNotFound: python2.6 + py27: commands succeeded + ERROR: pypy: InterpreterNotFound: pypy + ERROR: py32: InterpreterNotFound: python3.2 + py33: commands succeeded + docs: commands succeeded + pep8: commands succeeded + +You may not have all the required Python versions installed, in which case you +will see one or more ``InterpreterNotFound`` errors. + + +Explicit Backend Selection +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +While testing you may want to run tests against a subset of the backends that +cryptography supports. Explicit backend selection can be done via the +``--backend`` flag. This flag should be passed to ``py.test`` with a comma +delimited list of backend names. To use it with ``tox`` you must pass it as +``tox -- --backend=openssl``. + +Building Documentation +~~~~~~~~~~~~~~~~~~~~~~ + +``cryptography`` documentation is stored in the ``docs/`` directory. It is +written in `reStructured Text`_ and rendered using `Sphinx`_. + +Use `tox`_ to build the documentation. For example: + +.. code-block:: console + + $ tox -e docs + ... + docs: commands succeeded + congratulations :) + +The HTML documentation index can now be found at +``docs/_build/html/index.html``. + +.. _`pytest`: https://pypi.python.org/pypi/pytest +.. _`tox`: https://pypi.python.org/pypi/tox +.. _`virtualenv`: https://pypi.python.org/pypi/virtualenv +.. _`pip`: https://pypi.python.org/pypi/pip +.. _`sphinx`: https://pypi.python.org/pypi/Sphinx +.. _`reStructured Text`: http://sphinx-doc.org/rest.html diff --git a/docs/development/index.rst b/docs/development/index.rst new file mode 100644 index 00000000..0524ff0e --- /dev/null +++ b/docs/development/index.rst @@ -0,0 +1,17 @@ +Development +=========== +As an open source project, ``cryptography`` welcomes contributions of all +forms. The sections below will help you get started. + +File bugs and feature requests on our issue tracker on `GitHub`_. If it is a +bug check out `what to put in your bug report`_. + +.. toctree:: + :maxdepth: 2 + + getting-started + submitting-patches + reviewing-patches + +.. _`GitHub`: https://github.com/pyca/cryptography +.. _`what to put in your bug report`: http://www.contribution-guide.org/#what-to-put-in-your-bug-report diff --git a/docs/development/reviewing-patches.rst b/docs/development/reviewing-patches.rst new file mode 100644 index 00000000..13d9cd8f --- /dev/null +++ b/docs/development/reviewing-patches.rst @@ -0,0 +1,51 @@ +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: + +Architecture +------------ + +* Is the proposed change being made in the correct place? Is it a fix in a + backend when it should be in the primitives? + +Intent +------ + +* What is the change being proposed? +* Do we want this feature or is the bug they're fixing really a bug? + +Implementation +-------------- + +* Does the change do what the author claims? +* Are there sufficient tests? +* Has it been documented? +* Will this change introduce new bugs? + +Grammar/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? diff --git a/docs/development/submitting-patches.rst b/docs/development/submitting-patches.rst new file mode 100644 index 00000000..5978a1d6 --- /dev/null +++ b/docs/development/submitting-patches.rst @@ -0,0 +1,199 @@ +Submitting Patches +================== + +* Always make a new branch for your work. +* Patches should be small to facilitate easier review. `Studies have shown`_ + that review quality falls off as patch size grows. Sometimes this will result + in many small PRs to land a single large feature. +* Larger changes should be discussed on `our mailing list`_ before submission. +* New features and significant bug fixes should be documented in the + :doc:`/changelog`. + +If you believe you've identified a security issue in ``cryptography``, please +follow the directions on the :doc:`security page `. + +Code +---- + +When in doubt, refer to :pep:`8` for Python code. + +`Write comments as complete sentences.`_ + +Every code file must start with the boilerplate notice of the Apache License. +Additionally, every Python code file must contain + +.. code-block:: python + + from __future__ import absolute_import, division, print_function + +API Considerations +~~~~~~~~~~~~~~~~~~ + +Most projects' APIs are designed with a philosophy of "make easy things easy, +and make hard things possible". One of the perils of writing cryptographic code +is that secure code looks just like insecure code, and its results are almost +always indistinguishable. As a result ``cryptography`` has, as a design +philosophy: "make it hard to do insecure things". Here are a few strategies for +API design that should be both followed, and should inspire other API choices: + +If it is necessary to compare a user provided value with a computed value (for +example, verifying a signature), there should be an API provided that performs +the verification in a secure way (for example, using a constant time +comparison), rather than requiring the user to perform the comparison +themselves. + +If it is incorrect to ignore the result of a method, it should raise an +exception, and not return a boolean ``True``/``False`` flag. For example, a +method to verify a signature should raise ``InvalidSignature``, and not return +whether the signature was valid. + +.. code-block:: python + + # This is bad. + def verify(sig): + # ... + return is_valid + + # Good! + def verify(sig): + # ... + if not is_valid: + raise InvalidSignature + +Every recipe should include a version or algorithmic marker of some sort in its +output in order to allow transparent upgrading of the algorithms in use, as +the algorithms or parameters needed to achieve a given security margin evolve. + +APIs at the :doc:`/hazmat/primitives/index` layer should always take an +explicit backend, APIs at the recipes layer should automatically use the +:func:`~cryptography.hazmat.backends.default_backend`, but optionally allow +specifying a different backend. + +C bindings +~~~~~~~~~~ + +When binding C code with ``cffi`` we have our own style guide, it's pretty +simple. + +Don't name parameters: + +.. code-block:: c + + // Good + long f(long); + // Bad + long f(long x); + +...unless they're inside a struct: + +.. code-block:: c + + struct my_struct { + char *name; + int number; + ...; + }; + +Include ``void`` if the function takes no arguments: + +.. code-block:: c + + // Good + long f(void); + // Bad + long f(); + +Wrap lines at 80 characters like so: + +.. code-block:: c + + // Pretend this went to 80 characters + long f(long, long, + int *) + +Include a space after commas between parameters: + +.. code-block:: c + + // Good + long f(int, char *) + // Bad + long f(int,char *) + +Values set by ``#define`` should be assigned the appropriate type. If you see +this: + +.. code-block:: c + + #define SOME_INTEGER_LITERAL 0x0; + #define SOME_UNSIGNED_INTEGER_LITERAL 0x0001U; + #define SOME_STRING_LITERAL "hello"; + +...it should be added to the bindings like so: + +.. code-block:: c + + static const int SOME_INTEGER_LITERAL; + static const unsigned int SOME_UNSIGNED_INTEGER_LITERAL; + static const char *const SOME_STRING_LITERAL; + +Tests +----- + +All code changes must be accompanied by unit tests with 100% code coverage (as +measured by the combined metrics across our build matrix). + +When implementing a new primitive or recipe ``cryptography`` requires that you +provide a set of test vectors. + +Documentation +------------- + +All features should be documented with prose in the ``docs`` section. + +Because of the inherent challenges in implementing correct cryptographic +systems, we want to make our documentation point people in the right directions +as much as possible. To that end: + +* When documenting a generic interface, use a strong algorithm in examples. + (e.g. when showing a hashing example, don't use + :class:`~cryptography.hazmat.primitives.hashes.MD5`) +* When giving prescriptive advice, always provide references and supporting + material. +* When there is real disagreement between cryptographic experts, represent both + sides of the argument and describe the trade-offs clearly. + +When documenting a new module in the ``hazmat`` package, its documentation +should begin with the "Hazardous Materials" warning: + +.. code-block:: rest + + .. hazmat:: + +When referring to a hypothetical individual (such as "a person receiving an +encrypted message") use gender neutral pronouns (they/them/their). + +Docstrings are typically only used when writing abstract classes, but should +be written like this if required: + +.. code-block:: python + + def some_function(some_arg): + """ + Does some things. + + :param some_arg: Some argument. + """ + +So, specifically: + +* Always use three double quotes. +* Put the three double quotes on their own line. +* No blank line at the end. +* Use Sphinx parameter/attribute documentation `syntax`_. + + +.. _`Write comments as complete sentences.`: http://nedbatchelder.com/blog/201401/comments_should_be_sentences.html +.. _`syntax`: http://sphinx-doc.org/domains.html#info-field-lists +.. _`Studies have shown`: http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/ +.. _`our mailing list`: https://mail.python.org/mailman/listinfo/cryptography-dev diff --git a/docs/index.rst b/docs/index.rst index 19feb603..c8ef41b6 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -79,7 +79,7 @@ The ``cryptography`` open source project :maxdepth: 2 installation - contributing + development/index security api-stability doing-a-release -- cgit v1.2.3 From 450cd0252b4e8e53d574b18a5ad274c2ac32cca6 Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Tue, 11 Feb 2014 22:46:39 -0600 Subject: fix some indentation and a missing newline --- docs/development/getting-started.rst | 38 ++++++++++++++++++------------------ docs/development/index.rst | 1 + 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/docs/development/getting-started.rst b/docs/development/getting-started.rst index b367119e..d2d13a15 100644 --- a/docs/development/getting-started.rst +++ b/docs/development/getting-started.rst @@ -8,9 +8,9 @@ dependencies, install ``cryptography`` in ``editable`` mode. For example: .. code-block:: console - $ # Create a virtualenv and activate it - $ pip install --requirement dev-requirements.txt - $ pip install --editable . + $ # Create a virtualenv and activate it + $ pip install --requirement dev-requirements.txt + $ pip install --editable . You are now ready to run the tests and build the documentation. @@ -23,9 +23,9 @@ automatically, so all you have to do is: .. code-block:: console - $ py.test - ... - 62746 passed in 220.43 seconds + $ py.test + ... + 62746 passed in 220.43 seconds This runs the tests with the default Python interpreter. @@ -35,15 +35,15 @@ each supported Python version and run the tests. For example: .. code-block:: console - $ tox - ... - ERROR: py26: InterpreterNotFound: python2.6 - py27: commands succeeded - ERROR: pypy: InterpreterNotFound: pypy - ERROR: py32: InterpreterNotFound: python3.2 - py33: commands succeeded - docs: commands succeeded - pep8: commands succeeded + $ tox + ... + ERROR: py26: InterpreterNotFound: python2.6 + py27: commands succeeded + ERROR: pypy: InterpreterNotFound: pypy + ERROR: py32: InterpreterNotFound: python3.2 + py33: commands succeeded + docs: commands succeeded + pep8: commands succeeded You may not have all the required Python versions installed, in which case you will see one or more ``InterpreterNotFound`` errors. @@ -68,10 +68,10 @@ Use `tox`_ to build the documentation. For example: .. code-block:: console - $ tox -e docs - ... - docs: commands succeeded - congratulations :) + $ tox -e docs + ... + docs: commands succeeded + congratulations :) The HTML documentation index can now be found at ``docs/_build/html/index.html``. diff --git a/docs/development/index.rst b/docs/development/index.rst index 0524ff0e..c0272299 100644 --- a/docs/development/index.rst +++ b/docs/development/index.rst @@ -1,5 +1,6 @@ Development =========== + As an open source project, ``cryptography`` welcomes contributions of all forms. The sections below will help you get started. -- cgit v1.2.3 From 18962cc15b8562e46ea8b6f60fb5072c344d9b12 Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Tue, 11 Feb 2014 22:54:40 -0600 Subject: add explicit backend selection examples in a code block --- docs/development/getting-started.rst | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/development/getting-started.rst b/docs/development/getting-started.rst index d2d13a15..412f0545 100644 --- a/docs/development/getting-started.rst +++ b/docs/development/getting-started.rst @@ -55,8 +55,13 @@ Explicit Backend Selection While testing you may want to run tests against a subset of the backends that cryptography supports. Explicit backend selection can be done via the ``--backend`` flag. This flag should be passed to ``py.test`` with a comma -delimited list of backend names. To use it with ``tox`` you must pass it as -``tox -- --backend=openssl``. +delimited list of backend names. + + +.. code-block:: console + + $ tox -- --backend=openssl + $ py.test --backend=openssl,commoncrypto Building Documentation ~~~~~~~~~~~~~~~~~~~~~~ -- cgit v1.2.3 From 91c776fe1d577efa67beb4ea26caf9492bae4070 Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Tue, 11 Feb 2014 23:08:47 -0600 Subject: move around some material on the reviewing doc page --- docs/development/reviewing-patches.rst | 45 +++++++++++++++++++--------------- docs/spelling_wordlist.txt | 1 + 2 files changed, 26 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. + diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index cf421ea6..b258420f 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -3,6 +3,7 @@ backends boolean ciphertext committer +committers crypto cryptographic cryptographically -- cgit v1.2.3