code review is an architectural necessity
play

Code review is an architectural necessity Colin Dean @colindean 1 - PowerPoint PPT Presentation

Code review is an architectural necessity Colin Dean @colindean 1 @ColinDean Software Engineer Organizer, Abstractions.io Wearer of many hats 2 My words are my own and not my employer(s), past or present. Please save questions until the


  1. Code review is an architectural necessity Colin Dean @colindean 1

  2. @ColinDean Software Engineer Organizer, Abstractions.io Wearer of many hats 2

  3. My words are my own and not my employer(s), past or present. Please save questions until the end of the presentation. 3

  4. 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. 5

  6. Agenda • Quick anecdote • What is code review? • What problems do code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 6

  7. What is code review? 7

  8. 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. Code review is systematic examination of computer source code. Code Review , Wikipedia 9

  10. Peer Review 10

  11. Code Review 11

  12. 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 12

  13. The submitter proposes changes in a submission , which is evaluated by a reviewer , who annotates or accepts it. 13

  14. Most formal Least formal Team Pair Peer Inspection Walkthrough Ad-hoc review review programming deskcheck, passaround Wiegers’ peer review formality spectrum 14

  15. Most formal Least formal Team Pair Peer Inspection Walkthrough Ad-hoc review review programming deskcheck, passaround Wiegers’ peer review formality spectrum 15

  16. 16

  17. Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 17

  18. Aside from the primary goal of reducing defects, Code review solves two major problems. 18

  19. Code review solves Mental model synchronization 19

  20. 20

  21. 21

  22. On target Close enough Need guidance 22

  23. Code review solves Tribal knowledge development 23

  24. “ Architecture oral history requires that the team is both willing and able to retell the stories and keep the oral history alive.” Michael Keeling Creating an Architecture Oral History , SATURN 2012 24

  25. Code review forces us to Write it down. Make it searchable. 25

  26. Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 26

  27. Code review ensures Maintainability 27

  28. Code review drives Maintainability • Learnability • Understandability • Serviceability 28

  29. Code review drives Learnability • Developing Code • Patterns & Conventions • Risks & Goals • Developing People • Common Vocabulary • Teaching Moments 29 Maintainability Learnability Understandability Serviceability

  30. Learner Coding Reviewing Synchronous Exemplary Coding Pairing & Reading Teaching Expert Constructively Serendipitous Reviewing Critical Evaluation of Evaluation Example 30 Maintainability Learnability Understandability Serviceability

  31. Code review drives Understandability • Establishes common yet evolving mental model • Builds confidence in direction and design decisions • Builds tribal knowledge • Bonus: Enables elevator pitch 31 Maintainability Learnability Understandability Serviceability

  32. Code review drives Serviceability • Exposes addressable “gotchas” • Exposes end-user interaction points • Establishes consensus on supported workflows 32 Maintainability Learnability Understandability Serviceability

  33. “Given enough eyes, all bugs are shallow.” Linus’s Law 33

  34. Code review drives Maintainability ✓ Learnability ✓ Understandability ✓ Serviceability 34

  35. 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 • Continous integration easily circumvented 35

  36. Lack of code review Lost Opportunities 36

  37. Lost Opportunities Lack of code review Lost Revenue 37

  38. Lost Opportunities Lack of code review Lost Revenue Lost Job 38

  39. Code review ensures Compliance 39

  40. Code review drives Compliance • Accessibility • Auditability • Idiomaticity 40

  41. 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 modified and modifier only • No process of our own 41

  42. 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 42

  43. 43

  44. 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 44

  45. $1,450 per hour 45

  46. $1,450 per hour $5,800 per weekly meeting 46

  47. $1,450 per hour $5,800 per weekly meeting $290,000 per year 47

  48. Effects? • Waste • “Get this over with.” • Obstructionism • Plenty of bugs • “I’ll fix that mistake later.” 48

  49. 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 49

  50. Code review ensures Security 50

  51. Code review drives Security • Spot vulnerabilities • Teach best practices • Filter unnecessary code • YAGNI 51

  52. Reviewers are like your lawyer Screening and recommending actions to minimize risk, avoid preventable mistakes 52

  53. Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 53

  54. When should you integrate code review? 54

  55. Context • Project • Technical 55

  56. Keep reviews informal and short. 56

  57. Tips for thorough code review • Devote time • Accept debt • Identify churn • Minimize pedantry • Make progress 57

  58. Major things we look for • Algorithmic complexity • Style conformation (automate!) • Exception & error handling • Long lines & methods • Exception, class, & • Readability variable naming • Single purpose per • Logging sufficiency & commit level 58

  59. Most importantly Does it work? Is it tested? 59

  60. Agenda • Quick anecdote • What is code review? • What problems does code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 60

  61. Code review cannot Analyze dynamic structures 61

  62. Code review cannot Go on endlessly 62

  63. Code review cannot Solve political problems 63

  64. Agenda • Quick anecdote • What is code review? • What problems do code review solve? • Quality attributes code review ensures • Tips for code reviews • Limitations 64

  65. Code Review is systemic 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 . 65

  66. abstractions.io 1,500+ software professionals in Pittsburgh in August @abstractionscon 66

  67. @ColinDean github.com/ colindean/talks speakerdeck.com/colind ean 67

  68. FIN 68

Recommend


More recommend