Code reviews and how to survive them (from Froscon 2015)
How to get your patch acceptedLorna Mitchell, August 2015
View Slide
maintainer and team lead
unsolicited advice
stories
3 ingredients
code
words
perspective
what does it do?
feature/bug
ticket(s)?
If there aren't anyacceptance criteria for yourpatch, invent some
all the code
git diff master
git diff master...HEAD
tools
TOO BIG
common code mistakes
exotic code, no comments
Leaving commented code inyour codebase is like leavinglitter on a campsite
variable named $x
contributing.md
can you break your code?
I can
missing data
permissions
user is an idiot
tests
coding standards
syntax check
The commit history in yourbranch will be part of myproject until the end of time
git reset --soft $(git merge-base master HEAD)
more info than diff
accurate
sane format
http://chris.beams.io/posts/git-commit/
SFW
apologies
will it merge?
git merge --no-commit --no-ff featuregit merge --abort
fix it
rebase
A pull request is the openingline in a conversation abouta feature
source/target
diff
title
description
what will I observe?
why do I care?
code review
"review" vs "merge"
anyone
peer review vs gatekeepers
key skills
Being able to see whatyou're NOT looking at is areviewer's superpower
documentation
database patch
deployability
template
email
report
monitoring
cron job
and if not ...
reject
automated build
constructive feedback
colleagues vs contributors
too much feedback
sandwich technique
harsh feedback
Pull requests have alifecycle, be prepared tochampion them to the end
Thankyouhttp://lornajane.net(also speaking about PHP7 tomorrow)
lornajane
nagiss
nishiuma
finengine
defaultcf
hirosatogamo
koudaiii
miyashino
yrhorita
sansantech
soracom
colopl
torounit
eileencodes
jasonvnalue
pauljervisheath
addyosmani
wjessup
maggiecrowley
smashingmag
shlominoach
sonatard
notwaldorf
morganepeng