Skip to content

Commit

Permalink
Merge pull request #6 from dimagi/orangejenny-patch-1
Browse files Browse the repository at this point in the history
Added guidelines around timing
  • Loading branch information
orangejenny authored Sep 19, 2019
2 parents 5baaa21 + 896c65b commit 88c38a3
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,23 @@ At a very high level, the code review process should go as follows:
- Reviewers review the code and leave feedback for the author
- Once a reviewer has marked the PR approved, the author can merge the code.

In general, the onus is on the author to convince the reviewer that the changes are acceptable. However, the onus is on the reviewer to ensure that the changes requested are reasonable.
In general, the onus is on the author to convince the reviewer that the changes are acceptable. However, the onus is on the reviewer to ensure that the changes requested are reasonable.

In extremely rare circumstances where the author and reviewer cannot reach consensus, the review can be escalated, however this should be done in only the most extreme circumstances and should never be necessary if both the author and the reviewer are following the advice below.

Aim to submit your first review of a PR within one business day of the PR's creation, and to also submit subsequent reviews within a day of changes being made. Use your judgement as to when to break this guideline. Common considerations:
- Size: this turnaround is only possible when PRs are small enough that reviewing is a relatively short task.
- Time zones: if the author is finishing their workday soon and you have the capacity to review before that, you can potentially save days of back-and-forth communication.
- If you're deep into a long-term task and don't want to interrupt your focus to review, consider asking the author if the review can wait a few days, or if they can add another reviewer.
- As the author, it behooves you to communicate if (and why) your PR is urgent. It can also be a courtesy to indicate when a PR is low priority or is waiting on things besides review (business signoff, QA, etc).

## As the author

Reviewing code is hard, and as the author of code you should try your best to make the code as easy to review as possible. Here are some tips for doing that:

Read the page on [Writing PRs](https://github.com/dimagi/code-review/blob/es/writing-prs/Writing_PRs.md)

Also look at Google's guide to [getting through code reviews](https://google.github.io/eng-practices/review/developer/)
We largely follow Google's guide to [getting through code reviews](https://google.github.io/eng-practices/review/developer/)

### Responding to feedback

Expand All @@ -56,7 +62,7 @@ The author likely put a lot of work into the code that they wrote. As such it is
- For any “blockers” make sure that you communicate (respectfully) why they are blockers (see below).
- If you don’t feel like you have enough context to understand and merge the change, don’t hesitate to ask for an additional reviewer who has more context.

Also look at Google's guide to [how to do a code review](https://google.github.io/eng-practices/review/reviewer/)
We largely follow Google's guide to [how to do a code review](https://google.github.io/eng-practices/review/reviewer/)

### Blockers

Expand Down

0 comments on commit 88c38a3

Please sign in to comment.