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

▶
code review is an architectural necessity
SMART_READER_LITE
LIVE PREVIEW

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

May 15, 2023 36 likes •768 views

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

slide-1
SLIDE 1

Code review is an architectural necessity

Colin Dean @colindean

1

slide-2
SLIDE 2

@ColinDean

Software Engineer Organizer, Abstractions.io Wearer of many hats

2

slide-3
SLIDE 3

My words are my own and not my employer(s), past or present.

Please save questions until the end of the presentation.

3

slide-4
SLIDE 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

slide-5
SLIDE 5

5

slide-6
SLIDE 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

slide-7
SLIDE 7

What is code review?

7

slide-8
SLIDE 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

slide-9
SLIDE 9

Code review is systematic examination of computer source code.

9

Code Review, Wikipedia

slide-10
SLIDE 10

Peer Review

10

slide-11
SLIDE 11

Code Review

11

slide-12
SLIDE 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

slide-13
SLIDE 13

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

13

slide-14
SLIDE 14

Inspection Team review Walkthrough Pair programming Peer deskcheck, passaround Ad-hoc review

Wiegers’ peer review formality spectrum

14

Least formal Most formal

slide-15
SLIDE 15

Most formal Least formal

Inspection Team review Walkthrough Pair programming Peer deskcheck, passaround Ad-hoc review

Wiegers’ peer review formality spectrum

15

slide-16
SLIDE 16

16

slide-17
SLIDE 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

slide-18
SLIDE 18

Code review solves two major problems.

18

Aside from the primary goal of reducing defects,

slide-19
SLIDE 19

Mental model synchronization

Code review solves

19

slide-20
SLIDE 20

20

slide-21
SLIDE 21

21

slide-22
SLIDE 22

Close enough Need guidance On target

22

slide-23
SLIDE 23

Tribal knowledge development

Code review solves

23

slide-24
SLIDE 24

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

24

slide-25
SLIDE 25

Write it down. Make it searchable.

Code review forces us to

25

slide-26
SLIDE 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

slide-27
SLIDE 27

Maintainability

Code review ensures

27

slide-28
SLIDE 28

Maintainability

  • Learnability
  • Understandability
  • Serviceability

Code review drives

28

slide-29
SLIDE 29

Learnability

  • Developing Code
  • Patterns & Conventions
  • Risks & Goals
  • Developing People
  • Common Vocabulary
  • Teaching Moments

Maintainability Learnability Understandability Serviceability

Code review drives

29

slide-30
SLIDE 30

Learner Expert Coding Reviewing Coding Reviewing

Synchronous Pairing & Teaching Exemplary Reading Constructively Critical Evaluation Serendipitous Evaluation of Example

Maintainability Learnability Understandability Serviceability

30

slide-31
SLIDE 31

Understandability

  • Establishes common yet evolving mental model
  • Builds confidence in direction and design

decisions

  • Builds tribal knowledge
  • Bonus: Enables elevator pitch

Maintainability Learnability Understandability Serviceability

Code review drives

31

slide-32
SLIDE 32

Serviceability

  • Exposes addressable “gotchas”
  • Exposes end-user interaction points
  • Establishes consensus on supported workflows

Maintainability Learnability Understandability Serviceability

Code review drives

32

slide-33
SLIDE 33

Linus’s Law

“Given enough eyes, all bugs are shallow.”

33

slide-34
SLIDE 34

Maintainability

✓Learnability ✓Understandability ✓Serviceability

Code review drives

34

slide-35
SLIDE 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

slide-36
SLIDE 36

Lack of code review

36

Lost Opportunities

slide-37
SLIDE 37

Lack of code review

37

Lost Opportunities Lost Revenue

slide-38
SLIDE 38

Lack of code review

Lost Opportunities Lost Revenue Lost Job

38

slide-39
SLIDE 39

Compliance

Code review ensures

39

slide-40
SLIDE 40

Compliance

  • Accessibility
  • Auditability
  • Idiomaticity

Code review drives

40

slide-41
SLIDE 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

slide-42
SLIDE 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

slide-43
SLIDE 43

43

slide-44
SLIDE 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

slide-45
SLIDE 45

$1,450 per hour

45

slide-46
SLIDE 46

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

46

slide-47
SLIDE 47

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

47

slide-48
SLIDE 48

Effects?

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

48

slide-49
SLIDE 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

slide-50
SLIDE 50

Security

Code review ensures

50

slide-51
SLIDE 51

Security

  • Spot vulnerabilities
  • Teach best practices
  • Filter unnecessary code
  • YAGNI

Code review drives

51

slide-52
SLIDE 52

Reviewers are like your lawyer

Screening and recommending actions to minimize risk, avoid preventable mistakes

52

slide-53
SLIDE 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

slide-54
SLIDE 54

When should you integrate code review?

54

slide-55
SLIDE 55

Context

  • Project
  • Technical

55

slide-56
SLIDE 56

Keep reviews informal and short.

56

slide-57
SLIDE 57

Tips for thorough code review

  • Devote time
  • Accept debt
  • Identify churn
  • Minimize pedantry
  • Make progress

57

slide-58
SLIDE 58

Major things we look for

  • Algorithmic complexity
  • Exception & error

handling

  • Exception, class, &

variable naming

  • Logging sufficiency &

level

  • Style conformation

(automate!)

  • Long lines & methods
  • Readability
  • Single purpose per

commit

58

slide-59
SLIDE 59

Most importantly

Does it work? Is it tested?

59

slide-60
SLIDE 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

slide-61
SLIDE 61

Analyze dynamic structures

Code review cannot

61

slide-62
SLIDE 62

Go on endlessly

Code review cannot

62

slide-63
SLIDE 63

Solve political problems

Code review cannot

63

slide-64
SLIDE 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

slide-65
SLIDE 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

slide-66
SLIDE 66

1,500+ software professionals in Pittsburgh in August

abstractions.io

@abstractionscon

66

slide-67
SLIDE 67

@ColinDean

github.com/ colindean/talks speakerdeck.com/colind ean

67

slide-68
SLIDE 68

FIN

68

slide-69
SLIDE 69

Attributions

  • Westminster College picture: https://www.flickr.com/photos/westminstercollege/15759678054/in/album-72157649340620016/
  • RMU picture: http://cfbarchitects.com/higher-education/selected-projects/academic-buildings-libraries-learning-

commons/robert-morris-university/

  • Pittsburgh picture: probably Dave DiCello
  • On switch https://openclipart.org/detail/180085/switch-on
  • Off switch https://openclipart.org/detail/180084/switch-off
  • “Their first code review” http://classicprogrammerpaintings.tumblr.com/post/142702963264/their-first-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/

  • and others mentioned in the slides

69

slide-70
SLIDE 70

No, really. Fin. Srsly.

70

slide-71
SLIDE 71

Third out of school and current job - Engineering

  • Highly disciplined team using Java, Scala, and Groovy
  • Git + Gerrit
  • Constructively critical feedback
  • No criticism without alternative solution and reasoning
  • Wide experience range: 1-2 yrs to 25+ yrs
  • Team split in late 2014, I was asked to be tech lead

71

slide-72
SLIDE 72

Github Enterprise in 2016

  • All new projects
  • Same workflow as public Github

72

slide-73
SLIDE 73

Code Review Tools

Used Haven’t Used Like Dislike

★Github ★Gerrit ★Gitlab

  • Gitbucket
  • BitBucket

★StarTeam

  • Phabricato

r

  • git-assess

73