Code Reviews FTW!

About a year and half ago, we started to use code reviews in several teams in SUSE. What experience did we gain and what benefits do we see? How does a good review look like? What should a reviewer focus at and what should he/she avoid? And do code reviews make sense for you? We will try to answer all these questions.

Presented at WebExpo Prague 2013.

Details & video: http://webexpo.net/prague2013/talk/code-reviews-ftw/

Transcript

1. Prague, 21st September 2013 David Majda @dmajda Code Reviews FTW!
2. #1 Ugly Code?
3. #2 Hard to Start?
4. #3 Nobody Understands?
5. What? 1
6. Code review = code checked by other person
7. Why? 2
8. Code Quality
9. Encourage Learning
10. Bus Factor
11. SUSE Way 3
13. Code Change Pull Request Review Merge Accept Reject
14. My Way 4
15. Do I Understand It?
16. David's Personal Code Review Checklist
17. High-level Checklist
18. High-level Checklist ☑ Best approach & design?
19. High-level Checklist ☑ Best approach & design? ☑ Proper level of abstraction?
20. High-level Checklist ☑ Best approach & design? ☑ Proper level of abstraction? ☑ Change isolated?
21. Mid-level Checklist
22. Mid-level Checklist ☑ Already seen this?
23. Mid-level Checklist ☑ Already seen this? ☑ Easy to maintain?
24. Mid-level Checklist ☑ Already seen this? ☑ Easy to maintain? ☑ Easy to extend?
25. Mid-level Checklist ☑ Already seen this? ☑ Easy to maintain? ☑ Easy to extend? ☑ Not overengineered?
26. Mid-level Checklist ☑ Already seen this? ☑ Easy to maintain? ☑ Easy to extend? ☑ Not overengineered? ☑ Boy scout rule?
27. Low-level Checklist
28. Low-level Checklist ☑ Are erros handled?
29. Low-level Checklist ☑ Are erros handled? ☑ Covered by tests?
30. Low-level Checklist ☑ Are erros handled? ☑ Covered by tests? ☑ Follows coding style?
31. Low-level Checklist ☑ Are erros handled? ☑ Covered by tests? ☑ Follows coding style? ☑ Bugs: other occurrences?
32. Social Aspects 5
37. David Majda @dmajda