Add PR review process

Based on discussion in #8686, introduces a structured PR review process
that community members can follow. The PR process currently borne by
collaborators is split into two phases, review and merge, that can be
completed by different people thus allowing for better scalability.

Also replaces occurrences of "Pull-Request" in CONTRIBUTING.org with
"Pull Request" or "PR" for readability.
This commit is contained in:
Nikolai Myllymäki 2017-12-30 18:10:42 +02:00 committed by syl20bnr
parent 0d5f3cfe2f
commit 096a130b75
1 changed files with 44 additions and 5 deletions

View File

@ -7,7 +7,8 @@ contributors to follow.
You can only consider reading the sections relevant to what you are going to do:
- [[#asking-for-help][Asking for help]] if you are about to open an issue to ask a question.
- [[#reporting-issues][Reporting issues]] if you are about to open a new issue.
- [[#contributing-code][Contributing code]] if you are about to send a Pull-Request.
- [[#contributing-code][Contributing code]] if you are about to send a PR.
- [[https://github.com/syl20bnr/spacemacs/blob/develop/CONTRIBUTING.org#Reviewing-Pull-Requests][Reviewing Pull Requests]] if you are about to review a PR.
Thanks! :heart: :heart: :heart:
@ -18,7 +19,7 @@ Thanks! :heart: :heart: :heart:
- [[#general-contribution-guidelines][General contribution guidelines]]
- [[#license][License]]
- [[#conventions][Conventions]]
- [[#pull-request][Pull-Request]]
- [[#pull-request][Pull Request]]
- [[#ideally-for-simple-prs-most-of-them][Ideally for /simple/ PRs (most of them):]]
- [[#for-complex-prs-big-refactoring-etc][For complex PRs (big refactoring, etc):]]
- [[#commit-messages][Commit messages]]
@ -28,6 +29,7 @@ Thanks! :heart: :heart: :heart:
- [[#contributor-to-an-existing-layer][Contributor to an existing layer]]
- [[#contributing-a-keybinding][Contributing a keybinding]]
- [[#contributing-a-banner][Contributing a banner]]
- [[#reviewing-pull-requests][Reviewing Pull Requests]]
- [[#additional-information][Additional information]]
- [[#testing][Testing]]
- [[#credits][Credits]]
@ -83,14 +85,14 @@ Spacemacs is based on conventions, mainly for naming functions, keybindings
definition and writing documentation. Please read the [[https://github.com/syl20bnr/spacemacs/blob/develop/doc/CONVENTIONS.org][CONVENTIONS.org]] file
before your first contribution to get to know them.
*** Pull-Request
*** Pull Request
Submit your contribution against the =develop= branch. You should not use
your =master= branch to modify Spacemacs, this branch is considered to be
read-only.
You may want to [[https://github.com/syl20bnr/spacemacs/wiki/Beginner%27s-Guide-to-Contributing-a-Pull-Request-to-Spacemacs][read our beginners guide for Pull Requests]].
/PR = Pull-Request/
/PR = Pull Request/
**** Ideally for /simple/ PRs (most of them):
- Branch from =develop=
@ -206,7 +208,7 @@ file and don't require any contribution to Spacemacs.
If you think it worth contributing a new key bindings then be sure to read
the [[https://github.com/syl20bnr/spacemacs/blob/develop/doc/CONVENTIONS.org][CONVENTIONS.org]] file to find the best key bindings, then create a
Pull-Request with your changes.
PR with your changes.
*ALWAYS* document your new keybindings or keybindings changes inside the
relevant documentation file. It should be the layer's =README.org= file for
@ -222,6 +224,43 @@ If you have some ASCII skills you can submit your artwork!
You are free to choose a reasonable height size but the width size should be
around 75 characters.
* Reviewing Pull Requests
You can contribute by reviewing PRs created by others. This will help share the
workload of the project maintainers by letting them know that a PR has been
tested by an independent reviewer. The steps:
- Check that the PR complies with the guidelines in [[https://github.com/syl20bnr/spacemacs/blob/develop/CONTRIBUTING.org#contributing-code][Contributing code]].
- Check that the PR complies with [[https://github.com/syl20bnr/spacemacs/blob/develop/doc/CONVENTIONS.org][CONVENTIONS.org]].
- Check out the PR branch and test it. Remember to update your packages and your
=~/.spacemacs= file. Testing means that you actually use the features touched
by the PR, and the more complex or feature-rich the proposed changes are, the
more testing is required. Be creative in trying to find bugs! Preferably, use
the PR branch for hours or days to help stumble on unforeseen issues. Of
course, common sense can be used and typo fixes do not need to be tested
against bugs, but be thorough in actual code changes. Testing with a fresh
spacemacs installation might be a good idea as well.
- Step back and think if the proposed changes could cause any other problems not
covered by your testing. You should also ask yourself whether or not you feel
that your testing is adequate to confidently state that this PR introduces no
new bugs. If you feel that additional testing by more community members could
be helpful, state so in your review.
If you find something to improve, [[https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/][report]] it constructively and politely so the
contributor can update the PR accordingly. When you find that the PR is ready to
merge, you can leave an approving [[https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/][review]]. Please report explicitly how you
tested the PR for bugs, and confirm that you have checked its compliance with
the code conventions. Copy the following line to your approving review to notify
the collaborators:
#+BEGIN_EXAMPLE
Ready to be merged! (@syl20bnr @TheBB @d12frosted @bmag @JAremko)
#+END_EXAMPLE
Now the collaborators who have write access to the repository will use their
judgement to either merge the PR or require further review from another
reviewer. This is done to ensure a thorough cross-referencing in case of complex
changes, your review is very valuable in these cases as well!
* Additional information
** Testing
Tests live in the =tests/= folder, with a folder structure corresponding to the