Upgrade to Pro
— share decks privately, control downloads, hide ads and more …
Speaker Deck
Features
Speaker Deck
PRO
Sign in
Sign up for free
Search
Search
Get Your Patch Accepted
Search
Lorna Mitchell
August 22, 2015
Technology
1
1k
Get Your Patch Accepted
Code reviews and how to survive them (from Froscon 2015)
Lorna Mitchell
August 22, 2015
Tweet
Share
More Decks by Lorna Mitchell
See All by Lorna Mitchell
Introduction to OpenAPI Specification
lornajane
1
710
Create PDFs from markup with rst2pdf
lornajane
3
760
Serverless Microservices are the New Black
lornajane
3
110
Serverless Computing with Apache OpenWhisk
lornajane
0
100
Build A Serverless Data Pipeline
lornajane
1
750
SQL, NoSQL and Beyond
lornajane
0
1k
Build a Serverless Data Pipeline
lornajane
1
1.1k
Road Trip Through Database Country
lornajane
1
950
Serverless Microservices Are The New Black
lornajane
1
130
Other Decks in Technology
See All in Technology
AWSではじめる Web APIテスト実践ガイド / A practical guide to testing Web APIs on AWS
yokawasa
8
720
1行のコードから社会課題の解決へ: EMの探究、事業・技術・組織を紡ぐ実践知 / EM Conf 2025
9ma3r
12
4k
AI Agent時代なのでAWSのLLMs.txtが欲しい!
watany
2
230
"TEAM"を導入したら最高のエンジニア"Team"を実現できた / Deploying "TEAM" and Building the Best Engineering "Team"
yuj1osm
1
200
入門 PEAK Threat Hunting @SECCON
odorusatoshi
0
170
Apache Iceberg Case Study in LY Corporation
lycorptech_jp
PRO
0
340
【内製開発Summit 2025】イオンスマートテクノロジーの内製化組織の作り方/In-house-development-summit-AST
aeonpeople
2
700
AIエージェント時代のエンジニアになろう #jawsug #jawsdays2025 / 20250301 Agentic AI Engineering
yoshidashingo
8
3.8k
エンジニアリング価値を黒字化する バリューベース戦略を用いた 技術戦略策定の道のり
kzkmaeda
6
2.9k
What's new in Go 1.24?
ciarana
1
110
LINEギフトにおけるバックエンド開発
lycorptech_jp
PRO
0
310
JavaにおけるNull非許容性
skrb
2
2.6k
Featured
See All Featured
Navigating Team Friction
lara
183
15k
Speed Design
sergeychernyshev
27
810
Unsuck your backbone
ammeep
669
57k
Rails Girls Zürich Keynote
gr2m
94
13k
Building Adaptive Systems
keathley
40
2.4k
Cheating the UX When There Is Nothing More to Optimize - PixelPioneers
stephaniewalter
280
13k
RailsConf & Balkan Ruby 2019: The Past, Present, and Future of Rails at GitHub
eileencodes
133
33k
Sharpening the Axe: The Primacy of Toolmaking
bcantrill
40
2k
RailsConf 2023
tenderlove
29
1k
BBQ
matthewcrist
87
9.5k
Learning to Love Humans: Emotional Interface Design
aarron
273
40k
Fashionably flexible responsive web design (full day workshop)
malarkey
406
66k
Transcript
How to get your patch accepted Lorna Mitchell, August 2015
maintainer and team lead
unsolicited advice
stories
3 ingredients
code
words
perspective
what does it do?
feature/bug
ticket(s)?
If there aren't any acceptance criteria for your patch, 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 in your codebase is like leaving litter
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 your branch will be part of
my project 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 feature git merge --abort
fix it
rebase
A pull request is the opening line in a conversation
about a feature
source/target
diff
title
description
what does it do?
what will I observe?
why do I care?
code review
"review" vs "merge"
anyone
peer review vs gatekeepers
key skills
Being able to see what you're NOT looking at is
a reviewer's superpower
documentation
tests
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 a lifecycle, be prepared to champion them
to the end
3 ingredients
code
words
perspective
Thankyou http://lornajane.net (also speaking about PHP7 tomorrow)