Slide 1

Slide 1 text

PULL REQUESTS ARE LANGUAGE Abizer Nasir | @abizern | abizern.org I’m a contractor. Worked at many places Observations on Pull Requests

Slide 2

Slide 2 text

LANGUAGE AS COMMUNICATION Pull requests and code reviews as part of pull requests are language. Communicating within a team.

Slide 3

Slide 3 text

PRS ARE SERIOUS BUSINESS Pull requests are serious business This isn't something that one does as a chore alongside day to day work. It is part of the job.

Slide 4

Slide 4 text

A PULL REQUEST CAN ESTABLISH THE WHY, WHAT AND HOW OF A PROPOSED CHANGE Let’s deal with an easy bit first Raising a PR is describing the why, what, and how. Treat it like a paper or a proposal. The comment is the abstract. e.g. Fixes the transition bug by using an animation controller For those that raise the PR

Slide 5

Slide 5 text

THE PR QUEUE AS HIGH LEVEL OVERVIEW For those who consume the PR High level. What’s coming. Drill down to description for more. Look at code, whether reviewing or not Warning: staying on top of this means stand ups can either be better or worse, depending on whether everyone else does it. e.g. Did away with “what I did yesterday” part of standup.

Slide 6

Slide 6 text

Digression Before we get into it After a few apps where I was the sole developer, the first job I had with another programmer was with this guy.

Slide 7

Slide 7 text

”DON'T TOUCH MY SCREEN, DUDE!" We paired almost exclusively for 9 months No reviews Learned lots

Slide 8

Slide 8 text

▸ Nothing too trivial to question ▸ Code is easy. Thinking and planning is the real work ▸ Knowledge is shared through justifying decisions It's right in front of you, there is no context switching We spent a lot of time talking about what to do This was a real eye opener for me. Robust discussions.

Slide 9

Slide 9 text

STRONG OPINIONS, WEAKLY HELD This is an important takeaway. Be passionate, but open to new ideas.

Slide 10

Slide 10 text

CODE REVIEWS ARE SERIOUS BUSINESS Took this learning to jobs with code reviews Asynchronous Pairing, with obvious differences Strangely, this is the part that is controversial

Slide 11

Slide 11 text

- ESTABLISH CONVENTIONS - SHARE KNOWLEDGE - BUILD A TEAM with honest reviews come these wonderful opportunities. I don’t think for anything other than trivial changes, that a quick review from the diff is enough. Don’t just review changed lines. Checkout and review holistically You are talking about specific code, not generalised theory

Slide 12

Slide 12 text

ESTABLISH CONVENTIONS Alice reviews to Bob, who reviews to Candice who reviews Dani who reviews Alice e.g. guard over if-else

Slide 13

Slide 13 text

SHARE KNOWLEDGE Alice reviews to Bob, who reviews to Candice who reviews Dani who reviews Alice e.g. map over for loops e.g. Use animation controllers for transitions If many people are reviewing they become aware of other possibilities approaches that can be spoken about later. Don’t assume X is the expert, so this must be good. Ask questions

Slide 14

Slide 14 text

BUILD A TEAM Large scale design decisions not made in reviews No design survives first contact with the codebase Details are worked out in reviews consistent code, shared knowledge, discussions around specific design decisions Trust is developed. It’s about a good codebase People get to know who has what expertise or interest Discussion happens outside of code review Easier to ask for help if you feel you aren’t being judged Shared ownership

Slide 15

Slide 15 text

YOU ARE NOT YOUR CODE THEY ARE NOT THEIR CODE Remember this and you won’t go wrong This is hard if you’re a sensitive soul like me. Patterns change, you change. Don’t get attached. If someone points out something you could have done better, it’s a learning experience, not a slap down. Even if it is...

Slide 16

Slide 16 text

REVIEWING WRITING I try to review code as diligently as I write it I hope I’ve persuaded you to do likewise

Slide 17

Slide 17 text

THANK YOU Abizer Nasir | @abizern | abizern.org