Working on Pull Requests

The Abletech way

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.

Creating a pull request

• 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

Conducting a code review

• 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

Offering feedback

• 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

Responding to a feedback

• 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

Do’s and don’ts when working on a pull request

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