increasing code quality with code reviews (poetry version)

Increase Code Quality with Code Reviews

David Stockton SkiPHP Conference 2014

Increase Code Quality with Code Reviews

(Poetic Edition)

David Stockton SkiPHP Conference 2014

What is a Code Review?

A Code Review Is Making Your Code Better Than

It Would Be Without !

What is a code review?

A program looking at your

code

Skynet was started Code Review Reviewing Code

That was its own code


Looking at someone

else’s code

I see what you did Your code sucks worse than ever

Undo that crap now!


Someone else looking at your

code

I did that because I had a real good reason

I’ve forgotten it.

Common Ways to Do

Code Reviews

… And why those ways don’t work.

These are the things I’ve tried, I’ve tried These are the things

I’ve tried.

Ways to do Code Reviews

Email code or diffs

Code review email. Automatic filtering. Deleted. Unread.

Email Code Reviews

Create a diff, copy/paste code into an

email

This is what you do When you’ve too much time

on your hands And nothing really matters.

Anymore.

Mama Mia, Mama Mia, Mama Mia, let me go!

… and then?

Send to people you want to review your

code

Selecting my list My code review is sending

Sorry, reviewers.

Selecting my list My code review is sending

Sorry reviewers.

… and then?

They write back.

I got a reply! What did they think of my code?

I hate them, I think. :(

ERMAHGERD!

YER CODEZ SUCKZ!

My blood, sweat and tears Torn to ribbons in three words.

Your. Code. Sucks.

Reply back.

… Maybe.

I waited to hear. I should check my spam filter

Nope, it is empty.

Don’t you...

Response never comes.

There once was a man from Nantucket

Whose code reviews told developers to chuck it

They’d read it and weep And Crap code they’d keep

As they told the reviewer to go

Poem too long to fit, sorry...

… Forget about me

Or it’s ignored.

Expected response Got none for my code review Why should I bother?

You got me right in the feels

Text feedback taken in the most possible negative

way possible.

You said my code sucks Why do you h a t e me so much?

You are a huge jerk!

What a jerk!

From: Jake Developer!To: Byron Programmer!

Subject: Code Review for foo.php!!

Have you considered IOC for this? The current solution leads to a lack of extensibility which will probably

bite us later.!!

Also, two of the methods are missing doc comments and we need to put those

What a jerk!


Subject: Code Review for foo.php!

!

Have you considered IOC for this? The current solution

leads to a lack of

IOC? WTH does the International Olympic

Committee have to do with anything?

WTH does he know!



!


leads to a lack of

Lack of extensibility. Why don’t you write the stupid

thing yourself?

D to the B!



!


leads to a lack of

Bite Us? BITE ME!

...and the horse you rode in on!



!


leads to a lack of

Missing DOC comments? Pick many nits lately?



!


leads to a lack of

BALETED!

Picking my battles.

Ways to do code reviews

Evaluate Via Repo Diff

It’s just me… and me now

Usually done by one person

Lonely is the dev Reviewing Committed Code What is the point now?

Vacation? What’s vacation?

Requires Diligence

I checked in a LOT You didn’t say anything.

What is vay-cay-shun?

...and the bottle makes...

...and a small team

Didn’t we cover this?

Problems sent to dev via

email

Yes, yes we did.

See previous section for email

problems.

Yes, yes we did.

Second verse, same as the first.

There’s 24 hours in a day...

Large burden for single developer

Weight of the world is Is on my aching shoulders Must improve posture

It’s for your own good. I swear.

Developer usually very

senior to team

Why won’t everyone Recognize my super-brain!

I are the smartest!


Group Review in Conference

Room

Meetings are great tools To help everyone do

Just about nothing.

Movie night!

Gather developers

around a TV

I checked the reviews Rotten Tomatoes Gives It Code Review: Zero Stars

Worst plot evAr!

Time… is (not) on my side…

Takes up a lot of time

Meetings are great tools To help everyone do

Just about nothing.

People who enjoy meetings...

Yet another meeting.

…should not be in charge…

Developer can feel attacked.

Why’s everyone Always picking on me? Cuz I code like a chump And I don’t test anything

… of anything.

Needs a moderator

Just like herding cats So too is the job of the

Moderator guy.

After meetings, second favorite thing

Needs a note taker

Take note of this Said the young developer

Your code… Is bogus.

Why bother one person…

Often takes up the whole

team.

I was going to work But a code review says “No”

You will do no work.

…when you can bother everyone.

Not possible for all the changes.

Sometimes code is long A meeting will last too long Just let it go through?


Over-shoulder code review

I want to review But I cannot see your code.

Your hair is too big.

Developer in control

Please ignore that mess If you say nothing

Then it’s fine to go to prod.

Stop this review, I want to get off!

Reviewer Along for the

Ride

You asked me for help Where did the code go poorly?

You will not show me.

I hope that’s what you wanted to see

Developer leads reviewer through code

What I saw looked good…

Not much chance to

really review

I saw some good stuff The stuff you let me look at The rest must be fine.

Hey… Hey… HEY!

Interrupt the reviewer

Stop this code review I want to get off and look

What has changed in here?

We get to do that all again?

More than one reviewer

Oh, joy, what fun! And Now I get to do that all Back into the breach.

“I have way too much time” - No one

More time lost for developer

As I grow older and wiser I try to avoid

wasting time. - Bye. !

- SarcasmSociety


Pull Request Code Review

Not totally “teh suck”

One of the better options

Top-bottom diffs are Not as easy to read as Side to side code is.

</poetry>

Why you no take my code?

May require a pull-request

process

DVCS FTW!

Works great with git and mercurial

Sucky Version Numbers Crappy Version System

Less so with SVN or CVS

This is my rifle, this is a wrench...

Code review tools vary a

LOT


What does work?

Use the right tool for the job

Formal Code Review With

Software

Don’t hammer screws with a saw

Specifically Code Review

Software

It’s like they made it to do this.

Designed for Easy Review of Changes

Do it on your own time.

Reviews Don’t Interrupt

Development

If only I could write what I thought.

Comments on lines /groups

of lines

Comment all the things!

Comments on overall

changes

What are you going to say about it?

Allows for replies to comments

What about the code as a whole?

Overall Review Status

Stuff that has to be fixed

Issues

It’s all good

“Ship It”

Did you make the changes?

Allows for Updating Diffs

Code based on someone else’s code.

Diffs with Parent Diffs

Git, Hg, SVN, etc

Allow for Multiple VCS

Why don’t all things have APIs?

API to integrate into

process

How to Implement Formal Code Review

Installation / Hosted


Get buy-in by devs and

management


Decide how to integrate into

process.


How many reviews?


How many ship its?


Other requirements or restrictions


Timeframe to complete

review


Timeframe for responses

Code of Conduct for Reviews

No personal attacks


Really. No personal

attacks


Constructive criticism


Constructive criticism only.


No long ongoing

conversations


Move those to in-person, video, etc


Keep the review about

the code


What can be considered “issues”?

Benefits of Formal Code

Review

Benefits of Formal Code Review

Better Code


Bugs caught before launch or QA testing


Learning Opportunity for All Devs


More eyes leads to better

code

Software

• Review Board • Atlassian Crucible • Code Collaborator • Phabricator • Kiln • etc

Please rate this talk

https://joind.in/10448

increasing code quality with code reviews (poetry version)

Technology

code reviewsemail code

code sucks

lack of code review

code review email

code reviewsevaluate

elses code

code reviewwhy

code reviewsgroup review