Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Pull Requests are a Language

Abizer Nasir
September 29, 2023
4

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.

Abizer Nasir

September 29, 2023
Tweet

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. ▸ 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.
  8. 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
  9. - 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
  10. ESTABLISH CONVENTIONS Alice reviews to Bob, who reviews to Candice

    who reviews Dani who reviews Alice e.g. guard over if-else
  11. 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
  12. 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
  13. 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...
  14. REVIEWING WRITING I try to review code as diligently as

    I write it I hope I’ve persuaded you to do likewise