10 items to check before assigning a Pull Request to someone – Plus a BONUS

If you work in a team opening a Pull Request (or Merge Request) looks appropriate. It’s a common practice nowadays. However, have you ever thought about opening a Pull/Merge Request when working by yourself?

It may sound nonsense, but I do think it’s a good practice. There are several ways you can benefit from reviewing your own Pull Request. In this article, I wrote down how I do it in a few steps.

It’s great to count on colleagues to review your code correctly, find the blind spots, and share their knowledge with you. But it doesn’t matter whether you work by yourself or in a team, I recommend everyone to follow this list like a checklist:

Pick the correct target branch

Let’s start with the fastest and easiest item: check if your target branch is correct.

For this one, I would recommend taking a look at GitHub CLI and Hub. It allows you to create Pull Requests straight in your command line by passing a base branch:

$ hub pull-request -b my-target-branch

The command opens the editor set in $EDITOR environment variable, and all you need is to write Pull Request title and description.

Make Drafts explicit

I’ve seen several developers opening PRs when the work is mostly done. I recommend opening PRs in Draft right after the first commit. It lets you track your progress, feeds metrics tools, and get early feedback from your team or an automated code review service, like SourceLevel.

When writing the description, make a small checklist to make explicit how you’ve advanced in that change. It provides a clear idea of “next steps” for you and your peers. You can also use hub to open draft PRs with the -d option:

$ hub pull-request -b my-target-branch -d

Let’s talk about code

Let’s assume you opened a draft Pull Request to play with, what about the code? What should you review?

I recommend to start with low hanging fruits:

• Typo mistakes
• TODO or NOTE notations
• Comments
• Print/puts statements
• Trailing spaces or extra empty lines

You can configure your editor correctly to avoid the last item. Google by remove trailing spaces in <insert your favorite editor here>. Trust me; it saves a lot of time doing it automatically on saving a file.

Name things properly

Some developers spend a lot of time on naming modules, classes, variables, and so on. Usually, I start with a name that looks good. At the end of the development cycle, that name deprecates, and there may be better choices. Don’t forget to revisit your code and name them appropriately.

When you’re naming things, imagine you’re the thing. Put yourself in their shoes. Then check if the name that you’ve picked corresponds to the semantic of what you’re doing.

Ask help for tools

If your peers aren’t available all the time or work by yourself, you should benefit from Linters! They can be very handful.

Linters are tools that analyze your code to identify errors, warnings, style conventions, and complexity issues. For instance, for Ruby, there’s Rubocop; for Elixir, there’s Credo.

Using linters is also an excellent way to reduce and prevent technical debt in your codebase. Start with a few rules. Then keep adding appropriate rules to your linter config file.

However, you should always discuss those rules with your team before committing to them. Don’t do ignite a discussion in a Pull Request (unless it’s specific for that purpose). Move it to an issue where you can collect feedback from more team members and make sure that everyone is on the same page.

Remove the noise

When developing, I usually find some code to refactor, garden, correct style, or remove a useless parenthesis. For these cases, I recommend avoiding making changes that are out of context.

I know that most of the time, it’s hard to prevent yourself from fixing it. But, keep in mind that you can always open a separate Pull Request for those changes. In it, explain the reasons, and give additional details on why that change is essential.

Why should you care about avoiding unrelated code in PRs? Because it adds more complexity to the reviewer. More information makes the reviewing process less thorough. The less unrelated code, the more accurate and focused on the context the reviews are.

If you eventually make a change that you don’t want to discard, you should take a look at git stash.

Fetch necessary data

If changes involve fetching data from a database, check if you’re specifying only the columns that you need. By bringing unused data from the database, it may not be too expensive in some cases. However, keep in mind that it also requires more resources in the instances that fetch the data, retain more objects/structures, and increase memory consumption.

Understand Mergeability

Some PRs are more mergeable than others. How do I define whether a Pull Request is acceptable to merge?

Lots of teams require pull requests to pass a set of checks before they are eligible to merge. It’s interesting to increase the reliability of that change and ensure it won’t break when merged onto the target branch.

The checks usually include:

• Protected branches
• Required reviews
• Commit signing
• Linear commit history
• Required status checks

This last item should be a required step. The commits status checks indicate that integrated tools such as Continuous Integration and Automated Code Review checks are passing for each push. You can configure your repository to require each status checks to pass to merge.

Only green PRs are good to go.

Are you done?

Nice job! You arrived here and followed all the instructions. Now its time for the last but not least Pull Request metadata enhancements.

Pass the message

Essential pieces of information should fulfill your Pull Request description. They must guide your peers through the context of the changes. Do your best to explain why they’re there.

You should have checked all the items if you opened the Pull Request as Draft. Consider transforming it in a template for future Pull Requests. If you need a simple one, you can use SourceLevel’s Pull Request template.

More interesting stuff that you may include in description:

• “How to Test” instructions
• What is not part of this Pull Request (unrelated work)
• What can be improved in the future (consider creating an issue and referencing it in the description)
• Documentation and references
• List of references related to existing issues or other PRs
• Code snippets from other branches

Add screenshots

In case of including more than one screenshots, consider putting them into a <details> section, for instance:

<details>
<summary>Hidden screenshot</summary>

![alt text](<https://example.com/image.png>)
</details>

It toggles visibility and looks like this:

Example of a comment with a hidden screenshot

Feel free to use this snippet with GIF and video.

Be found in the future

People tend to forget that they can consult the PRs history. If you’ve never looked for an old PR, maybe you’re missing useful pieces of information.

Are you having trouble finding a specific PR? You should consider setting better labels to classify them. Do you need some helpful names? Check these:

• dependency
• documentation
• expedite
• feature
• good first patch
• ready for test, in test, test done
• security
• tech debt
• unplanned

Use your creativity to identify attractive labels that can help you and your team find issues and PRs.

Bonus advice: Comment inline in your changes

Comment inline any interesting fact or piece of code that you feed the urge to share. For example, you may point out a new API method that got you impressed with or a programming language feature that you’ve just learned.

You made it!

Congratulations 🎉! Did any changes since you’ve considered that Pull Request as “done”? Then rebase to make sure that everything is up-to-date and finally set the Reviewers.

Thanks for reading!

Did you learn something new by reviewing your own code? Share in the comments below!

The post 10 items to check before assigning a Pull Request to someone – Plus a BONUS appeared first on SourceLevel.