There's more to code review than you might think

There's more to code review than you might think

Lightning talk, given at PHPSC 2016

D887e6c4ecaca49b7add80a5f841727e?s=128

Daniel Shaw

June 11, 2016
Tweet

Transcript

  1. THERE’S MORE TO CODE REVIEW THAN YOU MIGHT THINK Clair

    Shaw @clairs · clairshaw.co.uk
  2. What are
 Code reviews for?

  3. Code reviews are not an indication
 of anybody's abilities

  4. Check code style

  5. Check code style Pick a house style, use it, communicate

    it
  6. Check code style Pick a house style, use it, communicate

    it
  7. Check code style Don't get too distracted by checking code

    style
  8. Check code style Automate if you can. Bad news can

    be better received by
 a cruel and lifeless script
  9. Review the configuration

  10. Review the configuration { "require": { "usefultool/tool": "1.1" } }

  11. Review the configuration { "require": { "usefultool/tool": "1.2.1" } }

  12. Review the configuration { "require": { "usefultool/tool": "^1.2" } }

  13. Check forwards compatibility

  14. Check forwards compatibility PHP 5.5.32
 PHP 5.6.18
 PHP 7.0.3

  15. Check forwards compatibility jQuery 1.12.1
 jQuery 2.2.1

  16. Review the documentation

  17. Review the commit message

  18. Review the commit message Update usefultool

  19. Review the commit message [MYCOOLAPP-1234] Update UsefulTool lib Wider company

    policy dictates that the latest UsefulTool v1 should be used. Update Composer config to reflect this.
  20. Double check the code

  21. Double check the code if (in_array($input, $expected)) { $this->showRestrictedSecrets(); }

    else { $this->goAway(); }
  22. Double check the code $expected = ['2', '4', '5', 'cake'];

    $input = 0; if (in_array($input, $expected)) { echo "I'm in!"; }
  23. Double check the tests

  24. Final thoughts…

  25. Final thoughts… All output should be reviewed.
 It’s not personal

  26. Final thoughts… Often, a peer review can teach
 something to

    two people
  27. Thanks. Feedback please! https://joind.in/talk/29752 
 clair@clairshaw.co.uk
 @clairs