Skip to content

Latest commit

 

History

History
71 lines (46 loc) · 2.7 KB

review.md

File metadata and controls

71 lines (46 loc) · 2.7 KB

Code review

We can improve the quality of the code base by reviewing changes before they are submitted.

Code reviews also serve to distribute liability, facilitating change.

We'll use Github to explore the idea of code review.

Create a review

Identify your reviewers using the review group generator for section 1:

$ java Generator [email protected] 1
[[email protected], [email protected], [email protected]]

In Github, fork each of your group's repositories.

Clone each forked repo.

In each, create a branch and implement a fix for the issue you reported earlier.

Push the branch to your forked repo.

Create a pull request and mention your reviewers.

For example, search Square's pull requests for usages of "cc" to see how people submitting pull requests ask for feedback from specific people.

Perform a review

If you are a reviewer, click through the “files changed” tab on the pull request to see the diff (example).

If you don’t own the repo you’re reviewing code for, just comment on the pull request saying if it looks good or not.

If you own the repo, and the change looks good, merge the change and close the issue.

Identify reviewers

Code review involves multiple people, which introduces organizational complexity. We can use the code review group generator to simplify this process. Random assignment ensures teammates maintain experience with the entire code base.

Alternatively, we can pre-identify owners of a code base and automatically add them to a review. Github uses "contributors" for this.

Some projects may define a two-step review process: first a member of my team reviews and then an owner approves.

Review checklist

We can use a checklist to improve consistency in our reviews.

For example:

  • Prefer consistency
  • Adhere to style guide and shared principles
  • Pay special attention to:
    • security-related changes
    • integrations with remote services
    • importing 3rd-party code
  • Localization
  • Accessibility
  • No dead code
  • Design review for significant changes
  • Reviewers participate in design review
  • Prefer smaller, milestone commits over monolithic changes
  • Maintain or improve test coverage
  • No surprising changes
  • Add recurring issues to this checklist and static analysis tools
  • Large changes must run locally

Learn more