Building a Code Review Culture

Jonathan Desrosiers


Information Services & Technology

External Affairs

External Affairs

Marketing & Communications


Interactive Design

Interactive Design

Jonathan Desrosiers

Web Developer III

First Assignment


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


Need To Fix

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



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

4. Improved Onboarding


  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 (for us)?

Our GIT Flow

  • No committing directly to the master branch.
  • All code must be added through a pull request.
  • Primary Branches must always be "production ready".
    • Plugins: `develop`
    • Themes: `master`
  • Merging code to `master` when not the primary branch requires a PR.

Our GIT Flow: Plugins

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

What Are Our Coding Standards?


Continuous Integration

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

Step 3:


Every Pull Request

Step 4:


BETA Testing

Additional Pull Request Rules

Limit PR Scope

Include a URL to Test

Avoid LGTM
(Looks Good To Me)

Make Suggestions

Delete Branches After Merging

Good Branch Naming

  • 12345-add-missing-sanitization-for profiles
  • fix/broken-admin-links-for-members
  • release/5.0

PR Responsibilities


Stalemate Rules

  1. All opinions should be documented in detail with reasoning on the PR.
  2. A third party will be called in to make a final decision.
    1. Which option is the most "futureproof"
    2. Does one approach prevent pivoting later if the chosen approach does not work?

Code Review Etiquette

Review Code Not the Person

Avoid You


Additional Success Factors


must buy in.

This is now a required part of the process.

Clearly Set Expectations

Everyone Reviews Code


Additional Processes

Pull Request & Issue Templates

Automatic Deployment to Staging

Visual Regression Testing


Everyone. Makes. Mistakes.

Thank You

Twitter: @Desrosj