Overcoming the stress surrounding Code Review, for the betterment of your project and career

Slide 1

Slide 1 text

Overcoming the Stress Surrounding Code Review, for the Betterment of Your Project and Career

Slide 2

Slide 2 text

Michael Zornek

Slide 3

Slide 3 text

What is code review?

Slide 4

Slide 4 text

No content

Slide 5

Slide 5 text

No content

Slide 6

Slide 6 text

How does it work?

Slide 7

Slide 7 text

master zorn/feature-123

Slide 8

Slide 8 text

Why is it beneficial?

Slide 9

Slide 9 text

Why is it stressful?

Slide 10

Slide 10 text

No content

Slide 11

Slide 11 text

Responsibilities of the Poster

Slide 12

Slide 12 text

Work is complete

Slide 13

Slide 13 text

The Reviewer is not your QA

Slide 14

Slide 14 text

PR is complete in detail

Slide 15

Slide 15 text

[Delivers #xxxxx] ## Finishes https://www.pivotaltracker.com/story/show/xxxxx ## Additions/Changes * One * Two * Three ## Testing Verify this. Use real hardware because of x.

Slide 16

Slide 16 text

As a user, I want FEATURE, So that I ACCOMPLISH_GOAL. Acceptance Criteria: * One * Two * Three

Slide 17

Slide 17 text

As a user, I want FEATURE, So that I ACCOMPLISH_GOAL. Acceptance Criteria: * One * Two Moved: #1234 * Three

Slide 18

Slide 18 text

Merge into master and kept up to date

Slide 19

Slide 19 text

Consider PRE commenting lines of code that are interesting

Slide 20

Slide 20 text

Responsibilities of the REVIEWER

Slide 21

Slide 21 text

Be timely in your review

Slide 22

Slide 22 text

Verify the behavior works as documented

Slide 23

Slide 23 text

Verify edge cases around the behavior

Slide 24

Slide 24 text

Review code quality

Slide 25

Slide 25 text

• Hard to comprehend • Single Responsibility Principal • Code Duplication • Boy Scout Rule • Off by One Errors • Large Input Errors • Error Handling • Performance • Old Devices • Low Memory • Method Names • Variable Names • Method Length • File Length • Docstrings • NSLocalizedStrings • Unit Tests • Testing Coverage

Slide 26

Slide 26 text

No content

Slide 27

Slide 27 text

Crafting productive criticism using plain text is hard

Slide 28

Slide 28 text

Style Nitpicks  (whitespace, indentation, etc.)

Slide 29

Slide 29 text

Praise!   Today I LearneD!

Slide 30

Slide 30 text

Too Many Cooks

Slide 31

Slide 31 text

Responsibilities of the POSTER

Slide 32

Slide 32 text

Be timely in your responses

Slide 33

Slide 33 text

Do not take the criticism personally

Slide 34

Slide 34 text

Don't be afraid to push back

Slide 35

Slide 35 text

Talk in person when needed

Slide 36

Slide 36 text

Crunch time reviewing

Slide 37

Slide 37 text

No content

Slide 38

Slide 38 text

Don't skip reviewing

Slide 39

Slide 39 text

Don't skimp reviewing

Slide 40

Slide 40 text

No content

Slide 41

Slide 41 text

You see, their morals, their code, it’s a bad joke. Dropped at the first sign of trouble. They're only as good as the world allows them to be. I'll show you. When the chips are down, these... these civilized people, they'll eat each other.

Slide 42

Slide 42 text

Accepts merges that… • add errors / stops the build • add warnings • break user behavior • are not associated with a ticket/story/sprint • have not been code reviewed • are not backed up by new tests • does not contain inline method documentation