A Code Review Review
Or: How Grown-ups Can Work Together Like Professionals to Build Software That Isn't Literally the Worst
I'm excited...
... so excited
... possibly too excited
... because reviews are my jam!
I know what you're thinking...
Really.
Here's why...
I hate being interrupted.
Like... I really fucking hate it
so much.
Interruptions
- Production Bugs
- Bullshit legacy code
- Surprise Requirements
- Manual regression testing
These are literally the worst.
They make engineers want to quit.
They are mostly preventable.
Better Peer Review
Success
A review is successful when everyone understands the motivation for the changes and can justify the approach.
Rules
Preparation
Feedback
1. Explain Everything
Get everything out of your head and out there for others to read. Often knowing someone's thought process is just important as their conclusions. Show your work.
2. Submitters determine the scope
It's up to the submitter to define what sort of feedback she is looking for and what the goal of the review is.
3. Reviewers determine the format
While it helps to agree upon how the feedback is formatted, it is ultimately up to the reviewers to decide how they provide their feedback.
4. Research > Ask > Assume
You will have questions. Try to find your own answers before peppering others with questions. Avoid assumptions at all costs.
Rules of Review
- Explain Everything
- Submitter decides the scope
- Reviewer decides the format
- Research > Ask > Assume
Preparation
Submitters
- Clean up your changes
- Review the changes yourself
- Set the goal of the review
- Provide a complete description of the problem
- Provide clear criteria for acceptance
- Check if reviewers have the resources they need
Reviewers
- Only review "clean" changes
- Demand a full description of the problem
- Demand clear acceptance criteria
- Delegate if you can't provide the right feedback (e.g. not enough time or expertise)
Feedback
Format
- Viewable alongside the changes
- Stored for reference and posterity
- Supports a conversation with several people at once
- Allow for asynchronous listening
Basically, just use pull-requests
Submitters
- Choose multiple reviewers
- Insist all feedback include supporting evidence and examples
- Challenge traditions; obey conventions
- Be open to learning
Reviewers
- Focus on addressing the type of feedback requested
- Qualify your feedback
- Don't deal in absolutes
- Know why you are making a recommendation and back it up
- Assume good intent and poor understanding
- Be open to learning
This Is Important
RetailMeNot engineering blog articles on this topic coming soon...