Skip to content

Commit

Permalink
Updated outdated information and reordered some points for better rea…
Browse files Browse the repository at this point in the history
…dability (#374)
  • Loading branch information
nikolay-bab authored Apr 15, 2020
1 parent 38bb6d0 commit cdca8a7
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions Etiquette/CODE_REVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@ Reviewing a Pull Request
* Do not comment if you don't have any suggestions to address your comment
* Use GitHub suggestions as much as possible
* If you feel strong against the change - request changes and explain why you are doing that. Do not request changes for typos, missing documentation comments or code style violations, trust your colleagues to address them.
* Approve changes if you are confident about them.
* If you are the first reviewer to approve the changes add the label "Needs one reviewer" so that others can see which PRs already have a feedback and which don't have any yet.
* Comment on the changes if something is not clear to you and needs further clarification or actions
* If you are not familiar with the domain ask for more context (better directly to the author) rather than skipping to something more familiar
* Ask, don't tell. ("What do you think about...?", "How about ...?", "Should we try ...?")
* Be brief but clear in your comments (code review is not about proving who is more correct or knows more)
* Unless explicitly asked for it do not suggest code to be written in a specific way - let the author to find or suggest a solution themselves, they may come up with a better one
* Submit all the comments as a single review, even if there is just one comment
* Approve changes if you are confident about them.
* If you are the first person to approve the changes, our bot will automatically add the label "Needs one reviewer" after your review.
* If you see a stale pull request without any updates - ask for the status update in the comment
* Do not be a blocker - if you requested changes and they were addressed make sure you review it
* Do not merge others pull requests - they may be waiting for someone else review. In general, the PR author should be the one adding the "Merge" label to merge their own PRs (except rare cases like if the author is on holidays but the PR is otherwise good to go)
Expand Down Expand Up @@ -96,7 +96,7 @@ PR Parties

To improve collaboration, we take time to review pull request together in small groups every week. This is what we call "PR parties".

- Each group should include at least one senior developer.
- Each group should include at least one senior developer.
- Each group will have a few pull requests assigned to them
- When done with their assigned PRs, the group is free to review something else (i.e. each other's pull requests) or finish the meeting

Expand Down

0 comments on commit cdca8a7

Please sign in to comment.