Automated Code Review

John Epperson

Eliminating the Gatekeeper

http://slides.com/kirillian/deck-3​​

Some Assumptions

  • Agile
  • Code Repositories
  • Continuous Integration

For the sake of brevity, I'm going to assume that you use, know about, or at least are familiar with the following topics:

Some of these things may have varying definitions depending on your place of work. If I say something weird that strikes you as odd. Please ask questions at the end or shoot me an email.

http://slides.com/kirillian/deck-3​​

Why do we want Code Reviews in the first place?

  • Consistent style
  • The meeting of code standards
  • Better, more manageable codebase

The Promise

http://slides.com/kirillian/deck-3​​

What is this Gatekeeper?

  • A single codereviewer for the app (or a small group)
  • A rule that all code must have a thumbsup from a senior developer

http://slides.com/kirillian/deck-3​​

Symptoms of the Gatekeeper System

  • Many unreviewed Pull Requests or outstanding branches depending on your Code Repository workflow.
  • One or more developers whose time is completely (or nearly so) tied up in reviewing these code pieces.
  • Long cycles between code completion and deployment.
  • Frustration with other reviewers' nitpicks
  • Inconsistency in rule and styling application

http://slides.com/kirillian/deck-3​​

Gatekeeper Alternatives

  • Abandon Code Review
  • Automation
  • Peer Review

http://slides.com/kirillian/deck-3​​

What is Peer Review?

  • Code Review is initiated by the author instead of being required 
  • Peers include Leads/Seniors/Mid-levels/and Juniors
  • Peer Review is focused on knowledge transfer, mentoring, and long-term sustainability

http://slides.com/kirillian/deck-3​​

How do we do it?

  • Developers MUST buy into this. You can't enforce or police a philosophy or idea. It will just be gamed.
  • You must make your team feel like peers. Juniors must feel like their opinions are wanted and Seniors must be open to taking the time out to teach.
  • You need to automate away as much of the process as you can. This includes style checks and code quality checks.

http://slides.com/kirillian/deck-3​​

Automation

The game-changer that makes peer review accessible to those Imperfect teams.

  • Consistency
  • Accuracy
  • Time Savings

Real

What it brings to the table:

http://slides.com/kirillian/deck-3​​

Automation Steps

  • Use some level of Continuous Integration (minimum: automated build/PR/commit checking)
  • Find a linter
  • Find a code quality checker
  • Use it and enforce the Green Light

http://slides.com/kirillian/deck-3​​

Examples:

Enter Rubocop

http://slides.com/kirillian/deck-3​​

> rubocop -D

Our First Run

http://slides.com/kirillian/deck-3​​

http://slides.com/kirillian/deck-3​​

> rubocop -aD --only Style/StringLiterals

Taking it in steps

http://slides.com/kirillian/deck-3​​

Done

http://slides.com/kirillian/deck-3​​

Enter Pronto

Running multiple tools on commit diffs

http://slides.com/kirillian/deck-3​​

The Reward

http://slides.com/kirillian/deck-3​​

What did we get?

  • Consistency of enforcement
  • Guarantee of meeting a standard
  • The promise of a better, more manageable codebase

http://slides.com/kirillian/deck-3​​

Some Other Tools

  • Ruby: Rubocop
  • Javascript: ESLint
  • Python: flake8

Linters

  • Ruby: Brakeman, Reek, Rails Best Practices
  • Javascript: JSHint

Code Quality Metrics

http://slides.com/kirillian/deck-3​​

Thanks for Listening

Full Version: http://slides.com/kirillian/deck

kirillian

john.epperson@rockeagile.io

http://slides.com/kirillian/deck-3​​