Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added guidelines around timing #6

Merged
merged 2 commits into from
Sep 19, 2019
Merged

Added guidelines around timing #6

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcf2a68
Added guidelines around timing
orangejenny Sep 19, 2019
896c65b
Update README.md
orangejenny Sep 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions README.md
View file
Open in desktop
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