My notes from the book “Looks Good to Me”, especially focusing on actionable steps.

Pull Request

1. Titles

The first the reviewer sees is the title, it should be short enough but enough to understand the answer of “what” the PR is about. On top of that, just like the Conventional Commits, you should prefix it with correct category.

feat(project or component name): short description

We should keep in mind to not add any unnecessary extra cognitive load on others like reviewer, and do our best to do things as easy as possible for them.

2. Description

Description should reply the question “why”. As the book points that these are optional, it shares this list with us.

feat: Context/Justification, Use Cases, How has this been tested?, Preview (if applicable), Documentation, Tests fix: Context/Justification, Linked Issue/Ticket, Before/After Fix Comparison, How To Test It, Documentation, Tests docs: Context/Justification, Linked Issue/Ticket, Use Cases, Preview (if applicable chore: Context/Justification breaking: Context/Justification, How has this been tested?, Affected Projects, Important Dates, Mitigation/ Next Steps, Documentation, Tests

As these are not certain rules, I feel like you can create your own template while using these as inspiration or copying them…

3. Use Labels

You can start by using simple labels such as “Feature” and “Bug”, then add new ones based on your use case. However, in case of labels quality > quantity, makes sure they makes sense and will actually help to clarify the PR.

4. Review States

Use review states to indicate the current status. Some review states that can be used, Draft, Ready for Review, In Review, Need Fixes, Approved.

5. Reviewer

Focus the code not the developer, don’t be lazy with your review and comments, they should be self-explanatory. If it’s complex, have a talk.

Whatever passes your review, it is your responsibility. You should automate the process as much as possible, if there’s not much automation you should check manually. The time you take should be 25-45 minutes, especially after 1 hour you won’t be able to focus much.

6. Author

First of all, review your own code before the PR, pretend like you’re the reviewer. If there are a lot of changes, it will be much harder to review, therefore try to keep it small and atomic, typically less than 500 lines and 20 files change. It should only include relevant changes. You’re not the code that you wrote, the feedback is not personal, take it positively.

If you send small, manageable, easy-to-understand PRs and review your own code before reviewer, the less negative feedbacks will happen, things will go more smoothly.

Review

1. Be objective

Looks good, but i prefer this one “abc”

Why is it better? Why this suggestion is better than my implementation? What is the reasoning behind of this? Before the suggestion, you need to ask yourself “why”. The comments must be objective, spesific, non-personal and clear. Could be based on data, convention, maybe you noticed an error, or it’s not completed based on acceptance criteria. It’s not about your preferences, if we get into that, everyone have their own and we start wasteful discussions.

2. Categorize comments

The book suggest two different methods for this. The first one is comment signals.

needs change: Small changes and fixes that the author can do in a small commit and can’t be ignored.
needs rework: Large changes and refactoring, most likely gets discussed offline.
align: Conventions that can’t be ignored.
levelup: Suggestions, improvements and feedback that can be ignored, but might be considered in a future PR.
nitpick: Subjective comments.

You can categorize your comments with MoSCoW as well: Must, Should, Could, Would.

Must: A must do change.
Should: A suggestion that should be discussed and agreed upon.
Could: Improvements but likely not part of the PR’s acceptance criteria therefore can be ignored.
Would: Personal preferences that can be ignored as well, can be used to guide to junior developers.

Lastly there is also Conventional Comments option as well.

The book author says that they failed to use MoSCoW because the lines between categories were blurry therefore they decided to use comment signals. These categories can be customized as you wish.

3. Comment Content

The book suggests Triple-R pattern, in case if you’re struggling to compose a helpful comment.

Request: What do you want the author to do? Rationale: Explaining why, with references and links if possible. Result: A measurable state that author can compare, could be metric, some screenshot of intended state or something about code.

4. Comment Tone

Use “we” instead of “you”. Using “you” makes it personal, the author may feel not good about it. The review is not about the author but the code.
Ask, do not command.
Sometimes you can compliment, but also avoid complimenting unnecessarily. You can compliment juniors more often though.