Building a Code Review Culture

Jonathan Desrosiers
@Desrosj

2016

Information Services & Technology

External Affairs

Information Services & Technology

  • Client Services & Support
     
  • Information Security
     
  • Projects, Communication & Administration
     
  • Enterprise Architecture & Systems
     
  • Educational Technology
     
  • Research Computing
     
  • Applications
     
  • Communication & Collaboration Services

Information Services & Technology

  • Client Services & Support
     
  • Information Security
     
  • Projects, Communication & Administration
     
  • Enterprise Architecture & Systems
     
  • Educational Technology
     
  • Research Computing
     
  • Applications
     
  • Communication & Collaboration Services

External Affairs

  • Marketing & Creative Services
     
  • External Affairs
     
  • BU Today
     
  • BU Productions
     
  • Government & Community Affairs
     
  • Federal Relations

External Affairs

  • Marketing & Creative Services
     
  • External Affairs
     
  • BU Today
     
  • BU Productions
     
  • Government & Community Affairs
     
  • Federal Relations

Marketing & Communications

  • Photography
     
  • Video
     
  • Interactive Design
     
  • Print
     
  • Editorial
     
  • Account Services
     
  • Public Relations

Marketing & Communications

  • Photography
     
  • Video
     
  • Interactive Design
     
  • Print
     
  • Editorial
     
  • Account Services
     
  • Public Relations

Interactive Design

Interactive Design

Jonathan Desrosiers

Web Developer III

First Assignment

Findings

  • Inconsistent code style.
     
  • Some missing best practices (uncached function calls, missed output escaping, etc.).
     
  • Performance improvements.

Expansion

Need To Fix

  • Inconsistent code style.
     
  • Some missing best practices (uncached function calls, missed output escaping, etc.).
     
  • Performance improvements.

Planning

Challenges

1. Many concurrent projects at a time.

2. Developers and designers write code.

3. Lots of variation in skills and experience.

4. Some projects are very old or long term.

Technical Factors

Human Elements

1. Opportunity To Learn

2. Opportunity For Knowledge Sharing

3. Collaboration

Steps

  1. Establish Rules
  2. Create a Process
  3. Document the Process
  4. Train Everyone

Step 1:

Establish Rules

1. Everyone must use the same code style.

2. Everyone must follow best practices.

3. Everyone must follow the same workflow.

4. Everything must be properly documented.

5. Everything must be accompanied by tests

(if appropriate).

Step 2:

Create a Process

What is the correct way to GIT?

Our GIT Flow

  • No committing directly to the master branch.
  • All code must be added through a pull request.
  • Plugins: develop is the primary branch.
  • Themes: master is the primary branch.
  • Master is always "production ready".

Our GIT Flow: Plugins

  • When a new version is ready, a PR from develop to master should be opened that:
    • Updates the changelog.
    • Increments the version number.

What Are Our Coding Standards?

Differences

Continuous Integration

How Do We Enforce Code Style, Best Practices, and Standards?

Step 3:

Documentation

Step 4:

Training

BETA Testing

Additional Tips

Update Changelogs

Limit PR Scope

Include a URL to Test

Avoid LGTM

Make Suggestions

Code Review Etiquette

Feelings

Avoid You

Engagement

Review Code Not the Person

Additional Success Factors

Everyone

must buy in.

This is now a required part of the process.

Clearly Set Expectations

Everyone Reviews Code

Bonus:

Additional Processes

Automatic Deployment to Staging

Visual Regression Testing

Learning

Thank You

Twitter: @Desrosj

Made with Slides.com