Code Review is an Architectural Necessity (Winter 2023)

Code Review is not a new concept, but a specific term for an older concept -- Peer Review -- that focuses the review process on the code under examination instead of the person who submitted it. While Code Review is common in open source projects, organizations developing in private often employ inefficient, frustrating, or virtually non-existent review processes. This talk looks at Code Review from an architect's viewpoint, arguing that Code Review is an essential component in a product's architectural plan because of the quality attributes it may provide. It also covers tips and sets expectations for what Code Review can and cannot solve.

(This version was closer to 50 minutes and features suggestions not included on previous editions.)

https://www.codemash.org/session-details/?id=380044

Transcript

1. #codereview Code review is an architectural necessity Colin Dean @colindean 1
2. @ColinDean Software Engineer Organizer, Code & Supply Director, Abstractions.io Wearer of many hats 2
3. #codereview My words are my own and not my employer’s, past or present. There may be some swearing. Please save questions until the end of the presentation. 3
4. #codereview Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 4
5. #codereview
6. #codereview Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 6
7. #codereview What is code review? 7
8. #codereview Code review is the process by which those who maintain a software codebase evaluate a proposed change to that codebase, regardless of the source of the proposed change. 8
9. #codereview Code review is systematic examination of computer source code. Code Review, Wikipedia 9
10. #codereview Peer Review 10
11. #codereview Code Review 11
12. #codereview Code Review ↳ Infrastructure Review ↳ Architecture Review ↳ Document Review 12
13. #codereview Code Review Vocabulary • Change - an individual unit of work altering what exists • Submission - a collection of changes • Submitter - the person proposing the submission • Reviewer - the people evaluating the submission • Annotation - remarks or ratings bestowed upon the submission 13
14. #codereview The submitter proposes changes in a submission, which is evaluated by a reviewer, who annotates or accepts it. 14
15. Inspection Team review Walkthrough Pair programming Peer deskcheck, passaround Ad-hoc review Wiegers’ peer review formality spectrum Least formal Most formal 15
16. Most formal Least formal Wiegers’ peer review formality spectrum Inspection Team review Walkthrough Pair programming Peer deskcheck, passaround Ad-hoc review 16
18. #codereview Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 18
19. #codereview Code review solves two major problems. Aside from the primary goal of reducing defects, 19
20. #codereview Mental model synchronization Code review solves 20
23. Close enough Needs guidance On target 23
24. Alternative 
 solutions New idea Open to 
 new idea 24
25. #codereview Tribal knowledge development Code review solves 25
26. #codereview Michael Keeling 
 Creating an Architecture Oral History, SATURN 2012 “Architecture oral history requires that the team is both willing and able to retell the stories and keep the oral history alive.” 26
27. #codereview Application code Code review audits 27
28. #codereview Application infrastructure Code review audits 28
29. #codereview Application architecture Code review audits 29
30. #codereview Write it down. Make it searchable. Code review forces us to 30
31. #codereview Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 31
32. #codereview Maintainability Code review ensures 32
33. #codereview Maintainability • Learnability • Understandability • Serviceability Code review drives 33
34. #codereview Learnability • Developing Code • Patterns & Conventions • Risks & Goals • Developing People • Common Vocabulary • Teaching Moments Understandability Serviceability Code review drives Maintainability Learnability 34
35. Learner Expert Coding Reviewing Coding Reviewing Synchronous Pairing & Teaching Exemplary Reading Constructively Critical Evaluation Serendipitous Evaluation of Example Understandability Serviceability Maintainability Learnability 35
36. #codereview Understandability • Establishes common yet evolving mental model • Builds con fi dence in direction and design decisions • Builds tribal knowledge • Bonus: Enables elevator pitch Serviceability Code review drives Maintainability Learnability Understandability 36
37. #codereview Serviceability • Exposes addressable “gotchas” • Exposes end-user interaction points • Establishes consensus on supported work fl ows Maintainability Learnability Understandability Serviceability Code review drives 37
38. #codereview Linus’s Law “Given enough eyes, all bugs are shallow.” 38
39. #codereview Maintainability ✓Learnability ✓Understandability ✓Serviceability Code review drives 39
40. #codereview First programming job out of school - B2B imprinting company if($customer == “spacely_sprockets”) { do_something(); } 
 else { cry(); } • Version control! • No code review tooling or process • Minimal pairing • Continuous integration easily circumvented 40
41. #codereview Lack of code review Lost Opportunities 41
42. #codereview Lack of code review Lost Opportunities Lost Revenue 42
43. #codereview Lack of code review Lost Opportunities Lost Revenue Lost Job 43
44. #codereview Compliance Code review ensures 44
45. #codereview Compliance • Accessibility • Auditability • Idiomaticity Code review drives 45
46. #codereview Second job out of school - Consulting • Lone wolf working alongside other lone wolves • No version control in proprietary software with custom “IDE” a.k.a. textarea. • Last modi fi ed and modi fi er only • No process of our own 46
47. First professional code review experience was group review • Subcontractor on government project, 2010-2012 • Lone SME on platform • Borland StarTeam + in house review system • My tools for version control integration • Weekly merge window • Round robin inspection
49. #codereview Not a pleasant experience • Three to four hour weekly round robin inspection • Cutthroat mixture of competing contractors, subcontractors, and employees • Embarrassment galore ‛ Not a learning environment • Immediate defensive posture • “Merge next week” = you failed, possibly delayed project 49
50. #codereview $1,450 per hour 50
51. #codereview $1,450 per hour $5,800 per weekly meeting 51
52. #codereview $1,450 per hour $5,800 per weekly meeting $290,000 per year 52
53. #codereview $1,450 per hour $5,800 per weekly meeting $290,000 per year 53 In 2010! ~$400,000 in 2023!
54. #codereview Effects? • Waste • “Get this over with.” • Obstructionism • Plenty of bugs • “I’ll fi x that mistake later.” 54
55. #codereview Missed opportunities • Accessibility expert was most vocal • Project manager was vocal on contractual and HF matters ➡ Both could have reviewed asynchronously • Project was behind ➡ Too many people could say No 55
56. #codereview Security Code review ensures 56
57. #codereview Security • Spot vulnerabilities • Teach best practices • Filter unnecessary code • YAGNI Code review drives 57
58. #codereview Reviewers are like your lawyer Screening and recommending actions to minimize risk, avoid preventable mistakes 58
59. #codereview Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 59
60. #codereview When should you integrate code review? 60
61. #codereview Context • Project • Technical 61
62. #codereview Keep reviews informal and short. 62
63. #codereview ±400 SLOC 63
64. #codereview Tips for thorough code review • Devote time • Accept debt • Identify churn • Minimize pedantry • Automate aggressively • Make progress 64
65. #codereview Major things we look for • Algorithmic complexity • Exception & error handling • Exception, class, & variable naming • Logging suf fi ciency & level • Style conformation (automate!) • Long lines & methods • Readability • Single purpose per commit 65
66. #codereview Most importantly Does it work? Is it tested? 66
67. #codereview Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 67
68. #codereview Analyze dynamic structures Code review cannot 68
69. #codereview Go on endlessly Code review cannot 69
70. #codereview Solve political problems Code review cannot 70
71. #codereview Agenda • Quick anecdote • What is code review? • What problems do code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 71
72. #codereview 72
73. #codereview Code Review is systematic examination of proposed changes to a codebase. solves mental model synchronization and tribal knowledge development. ensures maintainability, compliance, & security. must be short, thorough, and automated where possible. will not solve all human problems, but some is better than none. 73
74. #codereview codeandsupply.co the f ol ks behind @codeandsupply codeandsupply.fund 74
75. @ColinDean github.com/
 colindean/talks speakerdeck.com/ colindean 75
76. #codereview –Johnny Appleseed FINITA (that’s Esperanto for “ fi nished”) 76
77. #codereview Attributions • Westminster College picture: https://www. fl ickr.com/photos/westminstercollege/15759678054/in/ album-72157649340620016/ • “Their fi rst code review” http://classicprogrammerpaintings.tumblr.com/post/142702963264/their- fi rst-code-review- william-frederick • Bass, Len; Paul Clements, and Rick Kazman. Software Architecture in Practice. Addison Wesley, 2013. • Wiegers, Karl E. Peer Reviews in Software. Addison Wesley, 2012. • Cohen, Jason, Steven Teleki, and Eric Brown. Best Kept Secrets of Peer Code Review. Smart Bear Software, 2006. • Wilhelm, Alex and Alexia Tsotsis. Julie Ann Horvath Describes Sexism and Intimidation behind Her Github Exit. TechCruch, 2014 March 15. Retrieved 2016 April 26. http://techcrunch.com/2014/03/15/julie-ann-horvath- describes-sexism-and-intimidation-behind-her-github-exit/ • Baccehlli, Alberto and Christian Bird. Expectations, Outcomes, and Challenges Of Modern Code Review. Proceedings of the International Conference on Software Engineering, Proceedings of the International Conference on Software Engineering, May 2013. • Social media icons from FontAwesome • and others mentioned in the slides 77
78. #codereview BONUS SLIDES?! 78
79. #codereview Speci fi c tools for eliminating gotchas • pre-commit.com • github.com/sirwart/ripsecrets • Your editor’s Format on Save function • git-blame-ignore-revs 79
80. #codereview Speci fi c tools for process augmentation • ConventionalComments.org • CodeReviewChecklist.com 80
81. #codereview Practices • All programmers review • Healthy con fl ict is OK, as long as it’s not personal • Talk about code, not people • "This design doesn't allow…" vs "You didn't consider" • Immediate fi x vs backlog • Create tickets or commit TODO/FIXME/XXX comments 81
82. #codereview Other speakers with great Code Review material • Karl Wiegers • Jeff Atwood • Amy Gebhardt • Aaron Goldsmith • Adrienne Tacke 82
84. #codereview No, really. Fin. Srsly. Ne, reale. Finita. Serioze. 84