Pull Requests are a Language

A presentation at iOSCon 2018 in London about how Pull Requests are conversations between developers rather than just code reviews.

Transcript

1. PULL REQUESTS ARE LANGUAGE Abizer Nasir | @abizern | abizern.org I’m a contractor. Worked at many places Observations on Pull Requests
2. LANGUAGE AS COMMUNICATION Pull requests and code reviews as part of pull requests are language. Communicating within a team.
3. 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.
4. 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
5. 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.
6. 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.
7. ”DON'T TOUCH MY SCREEN, DUDE!" We paired almost exclusively for 9 months No reviews Learned lots
8. ▸ 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.
9. STRONG OPINIONS, WEAKLY HELD This is an important takeaway. Be passionate, but open to new ideas.
10. 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
11. - 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
12. ESTABLISH CONVENTIONS Alice reviews to Bob, who reviews to Candice who reviews Dani who reviews Alice e.g. guard over if-else
13. 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
14. 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
15. 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...
16. REVIEWING WRITING I try to review code as diligently as I write it I hope I’ve persuaded you to do likewise
17. THANK YOU Abizer Nasir | @abizern | abizern.org