Has Anyone else Seen

your code?

Codemotion Berlin 2018-11-21

Your Host Today

@pinchito

What We Will Cover


Bad Code Will Kill You


Deploying Spiders


Zen and the Art of Code Reviews


Editorial Process


Conclusions

Why Me?


20+ years in the industry


5+ years doing code reviews


Read a bunch of blog posts


Extremely muscular body

Code Can (and will) Kill You

Bad Code Is Out To Get You!


Software Bugs cost $60 Billion

Software handles:


  • all money except pocket change

  • communications around the world

  • health instruments and records

  • almost all transportation vehicles

state of the art sensors

vs primate in metal box

Ape wins

Self-Driving Car Manufacturers

Waymo
GM
Nissan
BMW
Tesla
...
Seat?
Dacia?
Perodua??
Saipa???

War Robots

Can be unleashed by a software bug

Actual robots

Deploying Spiders

Do you know what you deploy?


Often we do not know!

It may be weird stuff


(potentially very weird)

spoiler alert


It's a human head!

Does code ever work the first time?


Yes!

Probably not?

Definitely not

Hall of Tortured Souls

Linux 4.17.3: 6 MLOC


$ grep -ir fuck linux-4.17.3 | wc -l
29
$ grep -ir kludge linux-4.17.3 | wc -l
110
$ grep -ir cludge linux-4.17.3 | wc -l
1
$ grep -ir crap linux-4.17.3 | wc -l
195
$ grep -r TODO linux-4.17.3 | wc -l
4825
## Some Highlights

 * Wirzenius wrote this portably, Torvalds fucked it up :-)
/* !!!! THIS IS A PIECE OF SHIT MADE BY ME !!! */

Node.js 10.5.0: 3 MLOC


$ grep -ir fuck node-v10.5.0 | wc -l
25
$ grep -ir kludge node-v10.5.0 | wc -l
22
$ grep -ir crap node-v10.5.0 | grep -v scrap | wc -l
9
$ grep -r TODO node-v10.5.0 | wc -l
2904
## Some Highlights

* **help:** fuck it. just hard-code it ([d5d5085](https://github.com/zkat/npx/commit/d5d5085))
* IOW it's all just a clusterfuck and we should think of something that makes slightly more sense.

Java 10.0.1: 3.5 MLOC


$ grep -ir fuck java-10.0.1 | wc -l
1
$ grep -ir kludge java-10.0.1  | wc -l
16
$ grep -ir crap java-10.0.1 | grep -v scrap | wc -l
3
$ grep -r TODO java-10.0.1 | wc -l
2155
## Some Highlights
if (uri == null || uri.length() == 0) // crap. the NamespaceContext interface is broken
// forces us to clear out crap up to the next
* TODO: wrapping message needs easier. in particular properties and attachments.

Unit Tests are great

When someone reviews them!

External tests are awesome


But have to be repeated every time


Labor-intensive


Microsoft: 1 QA / dev in the 90s


Today: 1 QA / 3 devs

When Pair Programming is not enough

Even Remote Pair Programming

Zen and The Art


of code REview

Judged by a Jury of your Peers

Ideal Review

Do Not Rely Solely on Senior Review

 

Senior Review Creates

 a Choke Point

Mandatory Feynman Misquote


I couldn't explain my code to a junior. That means I don't really understand my code.

Richard Feynman

Encourage Juniors to Ask One Question

You may learn something
... or not

Too Many Prima Donnas Already

Ego-less Programming


TReat People Well


Do not give orders; ask questions


Maybe ask questions instead of giving orders?


Isn't it faster to change it than to argue?


Could you accept criticism gracefully?

Don't Troll People

Benefits


Knowledge is shared around


Disseminate coding culture


Status updates are much faster


It takes long, but not doing it takes longer

Explaining Your Code Is Good

If you don't learn anything you're doing it wrong

Volkswagen Scenario


Blame is spread


Other people reviewed


Go straight to the solutions

If Reviews Give You Trouble


Maybe you have some issues to work with?




Corollary:

If you want to improve your dev process, do reviews!

Editorial Process


Four-Eyes principle

Six Eyes

Eight eyes


Create Your Own Adventure!


Some examples


Node.js review


Apache consensus rules
3 +1s
no -1s
You can vote +0 or -0
Adjust as needed

Reviews are a process, not a destination

Prepare for multiple round-trips

To Summarize


People deploy Code all the time

"deploy"

Juniors Make Great Reviewers

Code Reviews Keep You Honest

Make The Process Explicit

Thanks!

@pinchito