Upgrade to Pro — share decks privately, control downloads, hide ads and more …

How To Do Positive Code Reviews

How To Do Positive Code Reviews

How to create an environment where code reviews are a positive experience for everyone involved.

Presented at PHP Cambridge on Feb 16th 2016.

Stuart Herbert

February 16, 2016
Tweet

More Decks by Stuart Herbert

Other Decks in Programming

Transcript

  1. @GanbaroDigital How Tasks Are Typically Done 1. Developer is assigned

    ticket in JIRA 2. Developer writes code 
 (and maybe some tests) 3. Best mate reviews code if they have the time 4. Developer merges code
  2. @GanbaroDigital How Tasks Are Typically Done 1. Developer is assigned

    ticket in JIRA 2. Developer writes code 
 (and maybe some tests) 3. Best mate reviews code if they have the time 4. Developer merges code
  3. @GanbaroDigital How Tasks Are Typically Done 1. Developer is assigned

    ticket in JIRA 2. Developer writes code 
 (and maybe some tests) 3. Best mate reviews code if they have the time 4. Developer merges code
  4. @GanbaroDigital How Tasks Are Typically Done 1. Developer is assigned

    ticket in JIRA 2. Developer writes code 
 (and maybe some tests) 3. Best mate reviews code if they have the time 4. Developer merges code
  5. @GanbaroDigital How Tasks Are Typically Done 1. Developer is assigned

    ticket in JIRA 2. Developer writes code 
 (and maybe some tests) 3. Best mate reviews code if they have the time 4. Developer merges code
  6. @GanbaroDigital How Tasks Are Typically Done 1. Developer is assigned

    ticket in JIRA 2. Developer writes code 
 (and maybe some tests) 3. Best mate reviews code if they have the time 4. Developer merges code
  7. @GanbaroDigital It is natural to seek out people who are

    more likely to agree with us. Especially in a toxic culture.
  8. @GanbaroDigital How Tasks Are Typically Done 1. Developer is assigned

    ticket in JIRA 2. Developer writes code 
 (and maybe some tests) 3. Best mate reviews code if they have the time 4. Developer merges code
  9. @GanbaroDigital How Tasks Are Typically Done 1. Developer is assigned

    ticket in JIRA 2. Developer writes code 
 (and maybe some tests) 3. Best mate reviews code if they have the time 4. Developer merges code
  10. @GanbaroDigital “Do as I say, not as I do.” How

    many of you have worked for someone like that?
  11. @GanbaroDigital Only then do you have the moral right to

    review the work of your colleagues.
  12. @GanbaroDigital Guiding Principles 1. The rule of no surprises. 2.

    Everything must stand up to scrutiny. 3. Everyone is entitled to a bad day. 4. Ask, don’t tell (sometimes).
  13. @GanbaroDigital 1. Make sure only the right people attend. 2.

    Make sure expectations are known in advance. 3. Discover and correct problems early.
  14. @GanbaroDigital Code Review Attendees 1. Person doing the work. 2.

    Their boss - team lead or senior dev. 3. Observers who need to learn.
  15. @GanbaroDigital Code Review Attendees 1. Person doing the work. 2.

    Their boss - team lead or senior dev. 3. Observers who need to learn.
  16. @GanbaroDigital Code Review Attendees 1. Person doing the work. 2.

    Their boss - team lead or senior dev. 3. Observers who need to learn.
  17. @GanbaroDigital Review Thrice, Ship Once 1. Review task and plan

    of attack. 2. Review code. 3. Review & approve code + tests.
  18. @GanbaroDigital Review Thrice, Ship Once 1. Review task and plan

    of attack. 2. Review code. 3. Review & approve code + tests.
  19. @GanbaroDigital Before Writing Code • Check understanding of the problem.

    • Review proposed solution. • Review proposed test plan. • Give or withhold approval.
  20. @GanbaroDigital Review Thrice, Ship Once 1. Review task and plan

    of attack. 2. Review code. 3. Review & approve code + tests.
  21. @GanbaroDigital After Writing Code • Review developed solution. • Review

    revised test plan. • Give or withhold approval.
  22. @GanbaroDigital Review Thrice, Ship Once 1. Review task and plan

    of attack. 2. Review code. 3. Review & approve code + tests.
  23. @GanbaroDigital Before Merging • Repeat test plan execution. • Review

    test plan execution results. • Give or withhold approval.
  24. @GanbaroDigital Make everyone 
 involved in a code review 


    accountable for everything that they have approved.
  25. @GanbaroDigital Give everyone 
 involved in a code review 


    the right to refuse to sign off the work.
  26. @GanbaroDigital Review Thrice, Ship Once 1. Review task and plan

    of attack. 2. Review code. 3. Review & approve code + tests.
  27. @GanbaroDigital No process can solve the problem of toxic people

    … … except perhaps the firing process!
  28. Would you like to 
 know more? We can help.

    Stuart Herbert ~ @stuherbert Founder @GanbaroDigital