Skip to content

Commit

Permalink
Merge pull request #2352 from jougs/userdocs
Browse files Browse the repository at this point in the history
Update review guidelines
  • Loading branch information
heplesser committed Apr 28, 2022
2 parents ef56996 + c60ccde commit 2c3b770
Showing 1 changed file with 34 additions and 21 deletions.
55 changes: 34 additions & 21 deletions doc/userdoc/contribute/code_review_guidelines.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,37 @@ meant to prevent progress, but to keep up the code quality as the
number of developers is growing. All of this is not set in stone and
can be discussed on the :ref:`NEST mailing lists <community>`.

* In general, the rule is that each pull request needs an OK from the CI
platform and at least two reviewers to be merged.
* For changes labeled “not code” or “minor” (e.g., changes in documentation,
fixes for typos, etc.), the release manager can waive the need for code
review and just accept the OK from the CI system in order to merge the request.
* Each pull request needs to be documented by an issue in the `issue
tracker <https://github.com/nest/nest-simulator/issues>`_ explaining the reason
for the changes and the solution. The issue is also the place for discussions
about the code.
* New features like SLI or PyNEST functions, neuron or synapse models need to
be accompanied by one or more tests written either in SLI or Python. New
features for the NEST kernel need a test written in SLI.
* Each change to the code has to be reflected also in the corresponding
examples and documentation.
* All source code has to be adhering to the Coding Guidelines for
:ref:`C++ <code_style_cpp>` in order to
pass the :ref:`continuous integration system <cont_integration>`.
* All commits should be coherent and contain only changes that belong together.
* Please check the **typeset documentation** as part of the review process. To
learn how to test the documentation locally offline, please check out our
:ref:`User documentation workflow <userdoc_workflow>`.
* In most cases, each pull request needs an OK from the
CI platform and at least two reviewers to be merged.
* The two reviews have to cover the technical side (i.e., if the code
does the right thing and is architecturally sound) and the content
side (i.e., if the code is scientifically correct and fixes an
actual issue).
* For changes labeled “not code” or “minor” (e.g., changes in
documentation, fixes for typos, etc.), the need for a second code
review can be waived, and a single review plus the OK from the CI
system is sufficient to merge the request.
* New features like SLI or PyNEST functions, neuron or synapse models
need to be accompanied by one or more tests written in either SLI or
Python.
* Each change to the code has to be reflected in the
corresponding examples and documentation.
* A pull request should be coherent and contain only changes that
belong together.
* Please also check that the typesetting of the documentation looks
correct. To learn how to test the documentation locally offline,
please check out our :ref:`User documentation workflow
<userdoc_workflow>`.


Before merging, reviewers have to make sure that:

1. pull request titles match the actual content of the PR and
be adequate for the release notes
1. pull request titles are complete sentences that start with an
upper-case, present-tense verb and end without punctuation
1. pull requests are assigned to projects and properly and completely
labeled
1. all discussions are settled and all conversations are marked as
resolved
1. there are no blocking issues mentioned in the comments

0 comments on commit 2c3b770

Please sign in to comment.