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.
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
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.
▸ 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.
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
- 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
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
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
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...