Culture Of Code Reviews

Most everyone will agree code reviews are an essential component of producing quality code. Yet many groups still do not perform code reviews as part of their development process. In this talk I will share what I have learned on reducing the pain of incorporating code reviews into the development process. In particular we will look at techniques for reducing the time code reviews take, addressing some of the social dynamics that surround code reviews and how to develop a team culture that self promotes code reviews.

Transcript

1. Lloyd Moore, President Lloyd@CyberData-Robotics.com www.CyberData-Robotics.com Culture of Code Reviews
2. Overview  What is the purpose of a code review?  Incorporating automation  What to look for  Team dynamics of code reviews  Hiring for code reviews  Managing conflict  Summary
3. What is the purpose of a code review?  Hint: This is one of my interview questions!  Catch bugs early & improve code quality  Promote team learning & cohesion in the code base  Grow as a developer
4. Automating Code Reviews  Coding style consistency  ALL code style issues should be reviewed by automation  C++ has some options here, other languages better:  Clang-format is a good choice  Cpplint  Custom development  Tool should fully enforce the team coding guidelines  Tool should be integrated into IDE/editor
5. Automating Code Reviews  Linters and static analysis tools cover more issues  Clang-tidy  Coverity  PVS Studio  Cpplint  Generally covers most common coding errors and known anti-patterns  The overarching goal for automation is to allow human reviewers to focus on more complex and domain specific issues
6. What To Look For  Each language has it’s own pitfalls  Each development domain has it’s own pitfalls  Each team has their own pitfalls  Look for these FIRST – they will be easist to spot as someone on the team has likely already gotten burned by it!
7. More What To Look For  Does the code implement the specification? (Implies there is a spec!)  Is the code readable?  Do comments line up with the code?  Is the code efficient for the task being performed?  Is the code testable?  There are TONS of things here that could be listed – but you get the point!
8. Team Dynamics Of Code Reviews  In my experience most code review pain is the result of poor team dynamics  Code reviews must be managed as a COOPERATIVE activity  Goal: Produce the best possible PRODUCT & TEAM
9. More Team Dynamics  Review comments can ONLY be directed at specific code  “This code sucks” = Not Acceptable!  Controversial items should be backed up with a best practice citation or research reference  Document items that become team standards  Comments should show the alternative approach
10. And More Team Dynamics  In cases where there is no clear answer it is the lead’s job to make the final determination  It is the team’s job to implement and respect that determination  If there isn’t enough evidence to determine the proper path without the lead having to step in – it isn’t that important!
11. What Team Dynamic Is Best?  Ultimately the team dynamic you want to create needs to match the type of development:  Research teams will have a different dynamic than production teams  Video game teams will have a different dynamic than teams building safety critical software  Embedded systems development will have a different focus than web development
12. Hiring For Code Reviews  Establishing the team dynamic starts when you hire the team!!!  Many people are adverse to code reviews as they have had bad experiences in the past  The interview should include questions on how the candidate perceives and manages code reviews  The candidate must AT LEAST be amiable to the team dynamic you are trying to create
13. Interview Questions  What is the purpose of a code review?  Catch bugs early, team cohesiveness, learning  Of course other answers are acceptable  Candidate should be able to communicate the value of the code review to the project, the team, and THEMSELVES  “I don’t do code reviews” - Yes I have actually gotten this answer!
14. Interview Questions  What do you look for in a code review?  This is pretty open ended and the response will depend greatly on the candidate's experience  Does the candidate have some set of things they look for, and is it reasonable?  If the only things listed are spelling and comments this is a warning sign they don’t know how to look at their own code
15. Interview Questions  What do you do if you are short on time and your manager asks you to skip the code review?  Credit to Jeff Gold for this one!  This is a tough one!! Needs to be asked in a “challenging” way.  Best answers will show balance of competing priorities, including challenging the manager  Best answers will show understanding that code reviews SAVE TIME  Worst answers involve the candidate either becoming directly argumentative or completely passive
16. But I Already Have A Team?!  Will have to go more slowly and incrementally change team culture  Likely will have some turn over and this is a good opportunity for new perspectives  Can also start by focusing on the team learning aspect only  As folks get comfortable with reviews they usually see the value and generate a virtuous cycle
17. Managing Conflict  Different developers are always going to have different ways they want to do things!  This will inevitability come out in code reviews  Generally it will be the team lead’s responsibility to guide the discussion
18. Managing Conflict  Key principles:  Drive for technical correctness – easy to defend  Drive toward accepted standards and best practices – easy to defend  CppCore Guidelines – checkers coming!  Drive toward team standards – easy to defend  If you cannot find a DEFENSIBLE reason to choose one or the other there may not be one – flip a coin and move on!  Unclear comment and variable name issues always go to the reader!
19. Summary  Code reviews are an integral part of being a professional developer and crafting quality code  Automate as much as possible  Reviewing code needs to be part of the team culture, not just a task  Hire to match the team culture  Manage conflict quickly and professionally
20. Questions?