1 of 38

Introducing peer review

Even if your boss is a git

2 of 38

About me

  • Ian Thomas
  • 5 years PHP at CWT
  • 2 years Drupal at Tui
  • Currently working in C# for Travelopia

3 of 38

Introduction

  • What is peer review?
  • Why use peer review?
  • How do you conduct a review?
  • How do you introduce reviews �to an existing team?

4 of 38

What is peer review?

“Peer review is the evaluation of work by one or more people of similar competence to the producers of the work.”

Wikipedia “Peer Review”

5 of 38

What is peer review?

“a disciplined engineering practice for detecting and correcting defects in software artifacts, and preventing their leakage into field operations”

Capability Maturity Model

6 of 38

Why use peer review?

“a favorable return on investment �for software inspections; �savings exceeds costs by 4 to 1”

National [USA] Software Quality Experiment

7 of 38

Why use peer review?

“Done well, it makes knowledge sharing and collaboration a standard part �of your development cycle.”

Ian Thomas

8 of 38

OK, you’ve convinced me. How do I get started?

9 of 38

You will need

  • A software project

10 of 38

You will need

  • A software project
  • Source control

11 of 38

You will need

  • A software project
  • Source control
  • Ticketing system (recommended)

12 of 38

You will need

  • A software project
  • Source control
  • Ticketing system (recommended)
  • Two or more developers

13 of 38

You will need

  • A software project
  • Source control
  • Ticketing system (recommended)
  • Two or more developers
  • Up front approval from your boss

14 of 38

You will need

  • A software project
  • Source control
  • Ticketing system (recommended)
  • Two or more developers
  • Up front approval from your boss

15 of 38

That’s fine, I have all that.

16 of 38

Per-ticket branching

  • Pick up a ticket.

17 of 38

Per-ticket branching

  • Pick up a ticket.
  • Create a new branch.

18 of 38

Per-ticket branching

  • Pick up a ticket.
  • Create a new branch.
  • Make your changes. Use as many commits as you need.

19 of 38

Per-ticket branching

  • Pick up a ticket.
  • Create a new branch.
  • Make your changes. Use as many commits as you need.
  • Switch branches when something urgent comes in.

20 of 38

Per-ticket branching

  • Pick up a ticket.
  • Create a new branch.
  • Make your changes. Use as many commits as you need.
  • Switch branches when something urgent comes in.
  • No impact on other developers.

21 of 38

I’ve finished my ticket. What now?

22 of 38

Pull requests

  • Opened when a developer finishes their ticket

23 of 38

Pull requests

  • Opened when a developer finishes their ticket
  • Sit in a queue until a reviewer is ready.

24 of 38

Pull requests

  • Opened when a developer finishes their ticket
  • Sit in a queue until a reviewer is ready.
  • Reviewer adds comments and may request changes.

25 of 38

Pull requests

  • Opened when a developer finishes their ticket
  • Sit in a queue until a reviewer is ready.
  • Reviewer adds comments and may request changes.
  • Developer addresses feedback and returns it to the reviewer.

26 of 38

Pull requests

  • Opened when a developer finishes their ticket
  • Sit in a queue until a reviewer is ready.
  • Reviewer adds comments and may request changes.
  • Developer addresses feedback and returns it to the reviewer.
  • Reviewer merges the pull request.

27 of 38

I’ve been asked to do my first review. ��Have you got any tips?

28 of 38

Tips for reviewers

Work as a team to build the best code base

Don’t make it a competition to find the smartest developer

29 of 38

Tips for reviewers

Use automated tools such as PHP Lint

Don’t nitpick over coding style

30 of 38

Tips for reviewers

Ask questions

Don’t pretend you know everything

31 of 38

Tips for developers

Remember their suggestions when you’re working on future tickets

Don’t be upset when a reviewer suggests changes

32 of 38

Tips for developers

Open followup tickets or ask for an intermediate review

Don’t ask for too much to be reviewed at once

33 of 38

Tips for developers

Ask a specific person if you need a prompt review

Don’t merge your own work

34 of 38

Tips for managers

Remember every defect found in peer review is a defect that wasn’t found by a customer

Don’t base appraisals on “number of defects found”

35 of 38

Tips for managers

Continue existing testing procedures

Don’t expect reviews to find all your problems

36 of 38

Sounds great, but my manager’s more interested in when the sprint will be finished.

37 of 38

Introducing it to the team

  • Be your team’s peer review champion.
  • Manager cares about cost and productivity.
  • Use a new starter as a guinea pig.
  • Retrospective: Look back at the review comments.
  • Was it useful?

38 of 38

Did I miss anything?