Similar to how we write sentences, there are a lot of ways to write code and produce the same outcome. Because each developer has their own coding style, ensuring the code quality is necessary before integrating it into the code repository. Code reviews provide a means to ensure that the quality of the code meets the standard of the team and identify any potential defects or improvements. Developers integrate this into a process called pull request (PR).
There are so many definitions one can find on the internet about what a pull request is, but essentially, a pull request is a review request. It is created for other contributors to examine the piece of code and add review comments. Often there are code updates, and once they are happy with the proposed changes, the code gets merged into the base branch.
Over several years of experience as a software developer who has worked with numerous teams, I’ve realised that each has its way of doing PR. While there are plenty of published resources about code styles and coding best practices, only very few talk about PRs. So in an attempt to standardise or establish consistency and completeness in creating and reviewing PRs, we, at Abletech, had a workshop about pull requests and how we do (or don’t do) it.
Use a helpful title and description — add the story ticket, give an overview, and add screenshots or videos for front-end changes
Link pull request to dependencies
Conduct a review of your own PR, similar to how you review other people’s work
Add a comment on architectural changes and new packages
Use clear and concise commits e.g. avoid using ambiguous commits like “Fixed based on PR feedback”
Ensure the request passed the CI pipeline
Give the reviewer a walk-through for difficult or long PRs
Well-indented code
Check the title and description, screenshots or videos
Check the number of files changed
Check the build status to see if it passed
Read existing comments
Check commit history
Check test coverage — ensure success and failure scenarios are covered
Leverage existing library code
Check if there are architectural changes not required for the PR
Consider security and performance
Check readability and coding styles
Mark files as viewed after review
Update the associated ticket
Pull branch and view locally when needed
Prioritize urgent PRs
Suggest an alternative approach to solving the problem. If available, provide links to documentation and give examples
Acknowledge good code. Add a thumbs up or positive feedback
Use constructive feedback
Use it as a learning tool. Be suggestive rather than directive
Give hints or discuss the suggested solution instead of giving the solution straightaway
Give questions that provoke thought. Open a discussion
Submit pull review then comment.
Observe the language used
Pair on complicated PR
Add a comment that says more than just approved when done
Don’t take it personally. Step back and don’t react right away
Treat it as a learning opportunity
Acknowledge feedback. Respond to comments
Mark as resolved/completed or thumbs up a review when done
Talk about the feedback you don’t agree with
Re-requesting review when feedback has been accommodated
Don’t use insult or inflammatory language
Don’t be offensive
Don’t hold off the PR
Avoid personal opinions
Avoid LGTM
Approve minor issue
Use static code analyser as part of CI
Approve PR you don’t agree with but create a ticket for technical debt
Use — fixup in commits that are related to an existing commit
Working with pull requests is not easy. Ensuring code quality is hard. There’s no written standard but this works for us. While all of these points may not necessarily apply to everyone, surely some of these are advantageous and suitable, if not practised yet.
read previous