Automated Code Review

John Epperson

A Look at the tools and the practice

Why do we want Code Reviews in the first place?

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

Code Review Problems

First Step: The Manual Code Review

def some_method
    do_something
end

Joe: 'Fix your spacing. It looks terrible'

  • Frustration
  • Resentment
  • Bad Blood

Second Step: Automation

  • Remove human element from the equation
  • Improve consistency
  • Spend less time on style comments

Enter Rubocop

Rails:
  Enabled: true

AllCops:
  Include:
    - '**/Rakefile'
    - '**/config.ru'
  Exclude:
    - 'berks-cookbooks/**/*'
    - 'bundler_stubs/**/*'
    - 'bin/**/*'
    - 'db/**/*'
    - 'config/**/*'
    - 'script/**/*'
    - 'tmp/**/*'

Metrics/LineLength:
  Max: 120

Style/AlignParameters:
  EnforcedStyle: with_fixed_indentation

Style/TrailingCommaInLiteral:
  EnforcedStyleForMultiline: comma

Let's Start Using It

Include or Exclude Files

Enable/Disable or configure individual cops

> rubocop -D

Our First Run

> rubocop -aD --only Style/StringLiterals

Taking it in steps

Done

Enter Pronto

Running multiple tools on commit diffs

Running Pronto Locally

> pronto run
> pronto run --index

Run pronto on the diff between your current branch and master

Run pronto on the diff between your index (staged files) and master

group :development, :test do
  gem 'pronto'
  gem 'pronto-brakeman', require: false
  gem 'pronto-rails_best_practices', require: false
  gem 'pronto-reek', require: false
  gem 'pronto-rubocop', require: false
end

Gemfile

Running Pronto on CI

machine:
  environment:
    PULL_REQUEST_URL: ${CI_PULL_REQUEST}
    PULL_REQUEST_ID: ${CI_PULL_REQUEST##*/}
    RUBOCOP_CONFIG: ./.rubocop.yml

test:
  override:
    - bundle exec pronto run -f github_status github_pr -c origin/master

circle.yml

Error

all:
  exclude:
    - 'spec/**/*'
github:
  slug: kirillian/sbm
  api_endpoint: https://api.github.com/
  web_endpoint: https://github.com/

max_warnings: 150
verbose: false

.pronto.yml

Success

WHY?!?!?

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

What this won't Fix

  • Lack of self-control
  • No tests
  • Procrastination

These Tools Work in These Environments

  • Opinionated and Strong Cultures
  • Strong Testing Cultures
  • Productive Cultures

Other Tools

  • Brakeman
  • Reek
  • Rails Best Practices
  • Linters (ES6, coffescript, etc)
  • Flay

Thanks for Listening

http://slides.com/kirillian/deck

kirillian

automated_code_review

By John Epperson

automated_code_review

  • 449