Best Practices for GitHub Pull Requests

Best Practices for GitHub Pull Requests

Pull Requests are there to save your day and your codebase! They are great and there are many features and best practices not everyone is aware of. To improve your work with Pull Requests for the future, I will give you a short overview of some best practices and hidden gems that will make your life easier.

Best Practices

There are some best practices (supported by GitHub features) that worked fine for me (and many others) in the last few years.

Keep it small

We all love to review pull requests with thousands of lines changed, don't we? No one likes countless lines to review. So keep it simple, this also means do not put all your changes in a single commit.

Since GitHub allows you to filter on a commit basis, it totally makes sense to group changes in commits. Let's assume you are doing a gigantic refactoring, which also includes deleting a few legacy files. You want to avoid creating a separate Pull Request, which is totally fine.

To keep it lean anyway, just commit the deletion as one single chunk, so it's straightforward to see what has been done here.

By grouping changes, especially large ones, inside a PR into commits is easy. But grants a much better visibility and makes the life of reviewers a lot easier. Don't worry about messing up your git history, it is just a pull request!

Better commit more frequently than just waiting until everything is done.

Make use of CODEOWNERS

As products change owners over time, it is best to have a code level ownership directly in the repository. GitHub makes this super easy with the CODEOWNERS-File. When you create a PR, GitHub will automatically add the right teams or even single persons as reviewers, so you won't need to look it up manually.

Source: https://github.blog/2017-07-06-introducing-code-owners/

You can also have multiple teams for file changes, so you can verify at least one of each of them approves your changes. All hassle-free, without you ever needing to do anything!

Open the PR after your first commit on the branch

What—are you crazy? This might be your first reaction, right? But there is a cool feature called “Draft Pull Requests”.

Source: https://github.blog/2019-02-14-introducing-draft-pull-requests/

This allows you to already get a first impression about your pull request, and you can adjust the description on the fly as you go on. Furthermore, you have a good feeling if the PR is just getting too big.

Once you are ready, Mark it as ready to review. Code owners and reviewers will be notified only after it is marked as such. So, you won't annoy anyone.

Choose the appropriate type for merging

Git provides more than just the “regular merge” which creates a merge commit. For most features, you want to squash your changes. This allows you to keep a clean git history, while not losing the contents of the commit. GitHub will add your commit messages in the body of the merge commit, and you have a single entry in your history. This way everything stays in shape, and you avoid unnecessary noise in the history.

Source: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request

Most of the time squash is what you want to do, especially when you are using conventional commits, this comes handy.

Write the title like a commit message

Authoring the title like this allows you to just use the heading as a commit message when merging. Furthermore, it provides a standardized title structure, which will help you keep a good overview over your pull requests and help people category the impact of your changes.

Summarize your changes

The description field is there for a reason, use it! Most people just leave it empty, but summarize your changes, describe the reasoning behind it. Even if you have a ticket in place, repeating the description in short here helps. Avoiding context switches is the goal here. The less work reviewers need to do, the more likely they will proactively review. This leads to shorter times you have to wait and therefore faster delivery.

A few bullet points are enough in most cases, for additional information or discussion that might come up you can use comments.

Leave feedback

Even if a PR looks good to you, outline what you liked. This helps people to stay motivated. It doesn't always have to be a comment about what has been done wrong!

Use the review feature wisely and add comments directly to the according lines, this way discussions are right next to the code and if everything is clear the author can just resolve the discussion.

It might seem un-personal to do this asynchronous, but it helps everyone to stay focused and review changes when it suits their timetable. This way you will get faster review times and better overall quality of reviews.

Use conventional commits and comments

Conventional commits are a well-defined structure how a commit should look like to contain everything that is relevant to readers of the git history.

Conventional comments basically take this approach and also use it for comments, this way no one feel offended by comments. Moreover, the priority is clear since everything is well-defined. It also has providing feedback (and of course praise) in mind.

Using conventions is something that has been proven to work across teams, companies, and even industries across the globe. So, why not using it in the git context as well?

You spend less time thinking about phrasing or how to write something, approximately 30% of your work has already been done, so you just need to use the right words ;)

Enforce standards using branch protection

Source: https://github.com/mc1arke/sonarqube-community-branch-plugin/issues/415

Branch protection is a super mighty feature allowing you to require a specific set of reviewers, status checks and many more.

So, you won't need to verify tests are passing or linting is fine, let tools handle this and focus in on the important things.

Focus on what's important

As previously mentioned, using conventional comments you can use conventional comments, and also set severity. A small nitpick doesn't prevent the PR to be merged. Besides approving, you can just leave a comment or request changes.

Choose these variants wisely, a critical bug fix shouldn't be prevented because the formatting in line 32 is missing an optional whitespace.

Agree in your team what you look for in pull requests. Usually checking the tests pass is NOT your responsibility, but the author's one.

You should focus on things that automation didn't cover, e.g., edge cases in formatting, best practices or most significant business/user requirements.

Conclusion

Pull requests should be fun, you should not be afraid of them. We all can make it a flawless process, with clear defined processes, common and well-known conventions like conventional commits & comments.

Automate what you can and focus on the rest manually. And most important: pull requests should have a high priority for everyone who is participating in the project! Keeping review times low and feedback useful helps everyone to stay productive. Otherwise, it quickly feels like it is just slowing you down.