The Art of Code Reviews

Slide 1

Slide 1 text

The Art of Code Reviews Fred @tfcporciuncula

Slide 2

Slide 2 text

The Art of Code Reviews Fred @tfcporciuncula Anita @anitas3791 David @_dlazaro Sebastiano @rotxed Lara @lariki Philipp @tehultrah

Slide 3

Slide 3 text

Code Reviews ! ! !

Slide 4

Slide 4 text

Some organizations do regular code reviews; those that don’t would do better if they did. Code reviews help spread knowledge through a development team. Reviews help more experienced developers pass knowledge to those less experienced. They help more people understand more aspects of a large software system. They are also very important in writing clear code. My code may look clear to me but not to my team. That’s inevitable — it’s hard for people to put themselves in the shoes of someone unfamiliar with whatever they are working on. Reviews also give the opportunity for more people to suggest useful ideas. I can only think of so many good ideas in a week. Having other people contribute makes my life easier, so I always look for reviews. Refactoring 2nd Edition — Martin Fowler

Slide 5

Slide 5 text

Slide 6

Slide 6 text

https:/ /twitter.com/relizarov/status/1123586265054699520

Slide 7

Slide 7 text

What this is *not* about • How to write comments • How to respond to comments • How to be empathetic and sensitive • How to not take it personally

Slide 8

Slide 8 text

What this is *not* about https:/ /github.blog/2015–01–21-how-to-write-the-perfect-pull-request/ How to write the perfect pull request http:/ /adavis.info/2018/09/frustration-free-code-reviews.html Frustration-Free Code Reviews

Slide 9

Slide 9 text

…and what this *is* about • What we do to make them more efﬁcient • The ideas behind our process • How to make your life easier • and much more! How the Blinkist Android team does code reviews

Slide 10

Slide 10 text

The WIP PR

Slide 11

Slide 11 text

The WIP PR

Slide 12

Slide 12 text

The WIP PR

Slide 13

Slide 13 text

The WIP PR 1. You do a tiny bit of work and open a PR 2. Everyone knows what you’re doing 3. People who care about what you’re doing can easily   4. Early feedback FTW keep an eye on it

Slide 14

Slide 14 text

The WIP PR 1. You do a tiny bit of work and open a PR 2. Everyone knows what you’re doing 3. People who care about what you’re doing can easily   4. Early feedback FTW keep an eye on it

Slide 15

Slide 15 text

The WIP PR

Slide 16

Slide 16 text

https:/ /github.blog/2019–02–14-introducing-draft-pull-requests/

Slide 17

Slide 17 text

https:/ /github.blog/2019–06–26-ask-students-to-iterate-with-draft-pull-requests/ The WIP PR

Slide 18

Slide 18 text

https:/ /github.blog/2019–06–26-ask-students-to-iterate-with-draft-pull-requests/ The WIP PR

Slide 19

Slide 19 text

The WIP MR

Slide 20

Slide 20 text

The WIP MR

Slide 21

Slide 21 text

The WIP MR https:/ /about.gitlab.com/2016/01/08/feature-highlight-wip/

Slide 22

Slide 22 text

The WIP PR

Slide 23

Slide 23 text

The WIP PR

Slide 24

Slide 24 text

The WIP PR https:/ /ben.straub.cc/2015/04/02/wip-pull-request/ The WIP Pull Request

Slide 25

Slide 25 text

Small PRs

Slide 26

Slide 26 text

Small PRs

Slide 27

Slide 27 text

No content

Slide 28

Slide 28 text

Squash? Merge? Rebase?

Slide 29

Slide 29 text

The ideal world

Slide 30

Slide 30 text

Squash? Merge? Rebase?

Slide 31

Slide 31 text

Squash and merge will make you lose your (beautiful?) commit history. But if your PR is small enough, then that’s ﬁne.

Slide 32

Slide 32 text

Commit-by-commit reviews

Slide 33

Slide 33 text

No content

Slide 34

Slide 34 text

No content

Slide 35

Slide 35 text

No content

Slide 36

Slide 36 text

A PR is like a presentation, and your slides are your commits.

Slide 37

Slide 37 text

No content

Slide 38

Slide 38 text

No content

Slide 39

Slide 39 text

No content

Slide 40

Slide 40 text

No content

Slide 41

Slide 41 text

No content

Slide 42

Slide 42 text

No content

Slide 43

Slide 43 text

No content

Slide 44

Slide 44 text

Commit-by-commit reviews It really helps with: • Presenting your changes in assimilable chunks • Unrelated, automatic changes and refactoring • Big PRs

Slide 45

Slide 45 text

The ideal world… doesn’t exist :(

Slide 46

Slide 46 text

Commit-by-commit reviews Two options to reach a commit-by-commit reviewable PR: • You’re can either build a beautiful commit history from the beginning • You rewrite your commit history before opening your PR

Slide 47

Slide 47 text

It’s ﬁne to have an artiﬁcial commit history, as long as it helps the review process.

Slide 48

Slide 48 text

Commit-by-commit reviews

Slide 49

Slide 49 text

• Rebasing while reviews are happening • “I'm ﬁxing that in my next commit" • It works better on change-intense PRs • It takes some time to get used to it Commit-by-commit reviews

Slide 50

Slide 50 text

“I'd never go back!” — Someone from the Blinkist team

Slide 51

Slide 51 text

Extra bits

Slide 52

Slide 52 text

Extra bits 1. Create great descriptions 2. Have a good PR template

Slide 53

Slide 53 text

No content

Slide 54

Slide 54 text

No content

Slide 55

Slide 55 text

Extra bits 1. Create great descriptions 2. Have a good PR template

Slide 56

Slide 56 text

Extra bits 4. Embrace self-reviews 5. Keep PR list clean 6. Emoji driven comments " 1. Create great descriptions 2. Have a good PR template

Slide 57

Slide 57 text

# driven comments ❤ – Praise and compliment % – Suggest ideas ❓ – Ask questions ' – Nit-picky comments ⚠ – Warn and express concern ) – Blocker!

Slide 58

Slide 58 text

# driven comments

Slide 59

Slide 59 text

# driven comments

Slide 60

Slide 60 text

# driven comments

Slide 61

Slide 61 text

# driven comments * * *

Slide 62

Slide 62 text

Summary • WIP PRs • Small PRs • Commit-by-commit reviews • Keep things clean (and maybe use %, ❓, ')

Slide 63

Slide 63 text

Good pull requests are an act of empathy, for both the author and reviewer.

Slide 64

Slide 64 text

Good pull requests are an act of empathy, for both the author and reviewer. https:/ /slack.engineering/on-empathy-pull-requests-979e4257d158