Code Reviews

That's a Great Idea

Amy Gebhardt ∙ MidwestPHP

Hi, I'm Amy.

@amlyhamm
Lead Front-End Software Engineer
Maine / Boston / Minneapolis
ENFJ

"Write the abstract of the talk you wish to attend"

What are Code Reviews?

Why are they worth it?

What should you be reviewing?

Who should be involved?

When should they happen?

How should reviews happen?

what are code reviews

And why are they useful?

The part in the software development process where humans evaluate another human's code.

Software is written
by humans.

Humans make mistakes.
Software has mistakes.

The Code Review Guarantee

Catch mistakes in the least-costly of development times.
Ensure the team is held accountable to quality standards and expectations.
Share domain knowledge across your entire team.
Provide opportunities to learn, mentor, and teach.
Build a stronger
foundation of trust.
"effectiveness of code review for determining faults in software is between 30% and 35% more effective than standard unit testing"
- Steve McConnell, Code Complete

Ok, now what?

Code reviews are like marriage.

One size does not fit all.
You and your team
get to make up the rules.

what should you be reviewing?

Code, right? For bugs?
what should you be reviewing?

Code you did not work on.

About 200-400 lines of code.

Timebox it to about an hour.

"Finding bugs in code is hard. Finding bugs in someone else's code is even harder."
- Jim Bird, Don't Waste Time on Code Reviews

Create a checklist of things to review.

what should you be looking for?

CORRECTNESS

CLARITY

EFFICIENCY

STYLE

CORRECTNESS

Does it do what it's supposed to do?
Off-by-one
Copy & paste / Spelling
Edge cases & exception handling
Data validation & security

CLARITY / MAINTAINABILITY

Can someone other than the author describe what's going on?
Self-documenting naming conventions
Function length & purpose
File length & purpose
Ternary operators, Guard Clauses, etc

EFFICIENCY

What's the performance impact?
Database queries
DOM manipulation
Asynchronous JavaScript
Using an ORM
Library usage

STYLE

Does this meet your style standards?
(hint: you should have style standards)
Comments
Line length
Naming conventions
Indentation

Don't spend your time manually reviewing what a linter can find.

who should be involved?

The intern? The CTO? The whole team?
Keep the number small.
You don't need everyone there to get the benefits of a review
Don't be afraid to review code you aren't familiar with
There's great value in a fresh perspective!

when should
reviews happen?

Literally all the time, always?
When should they happen?
Every commit?
Every branch merge or pull request?
Spontaneous/unscheduled?
Weekly?
Before/after each sprint review?

Make reviews part
of the culture.

Make it an essential step in your coding process.
Treat it like unit testing,
refactoring, or QA.

How should
reviews happen?

One-on-One? Using an online tool?

Online
vs.
In-Person

Online Tools
Crucible - Atlassian Suite
atlassian.com/software/crucible
Upsource - Jetbrains
jetbrains.com/upsource/
Pull Requests - GitHub
github.com/features

Now you're all
set, right?

Those soft skills?
They matter.

Have a style guide.

Decide what's important and don't
waste your time on what's not.

Avoid general feedback.

It sucks. Always.

General Negative Feedback :(

"this is messy"
"lolwut"
"you never want
to append in a loop"

General Positive Feedback :(

"this looks great"
"yay!"
":party-parrot:"

Specific Negative Feedback :)

"this function is missing
a return type"
"could we use an ES6 promise here to avoid an extra digest cycle?"

Specific Positive Feedback :)

"Yesss happy to see
flexbox being used!"
"The additional describe blocks really help organize our tests"

Assume
positive intent.*

Trust your teammates. Be respectful. Be humble.
*huge disclaimer

Have expectations for conduct.

Call each other out when
someone violates it.

It's about making
code better.

X

It's about
empowering
each other to write really great code.

Just try it?

Code Reviews
That's a Great Idea
CodeMash ∙ Amy Gebhardt

Questions or Comments?

Thanks for attending!