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 and . It allows you to create Pull Requests straight in your command line by passing a base branch: GitHub CLI Hub $ hub pull-request -b my-target-branch The command opens the editor set in environment variable, and all you need is to write Pull Request title and description. $EDITOR Make explicit Drafts I’ve seen several developers opening PRs when the work is mostly done. I recommend 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 . opening PRs in Draft 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 to open draft PRs with the option: hub -d $ 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 or notations TODO NOTE Comments Print/puts statements Trailing spaces or extra empty lines You can configure your editor correctly to avoid the last item. Google by . Trust me; it saves a lot of time doing it automatically on saving a file. remove trailing spaces in <insert your favorite editor here> 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 ! They can be very handful. Linters Linters are tools that analyze your code to identify errors, warnings, style conventions, and complexity issues. For instance, for , there’s ; for , there’s . Ruby Rubocop Elixir Credo Using linters is also an excellent way . Start with a few rules. Then keep adding appropriate rules to your linter config file. to reduce and prevent technical debt in your codebase 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). where you can collect feedback from more team members and make sure that everyone is on the same page. Move it to an issue 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 to merge. status checks to pass 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 . Consider . If you need a simple one, you can use SourceLevel’s . Draft transforming it in a template for future Pull Requests 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 section, for instance: <details> Hidden screenshot ![ ]( ) < > details < > summary </ > 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 to make sure that everything is up-to-date and . rebase finally set the Reviewers Thanks for reading! Did you learn something new by reviewing your own code? Share in the comments below! Previously published at https://sourcelevel.io/blog/10-items-to-check-before-assigning-a-pull-request-to-someone-plus-a-bonus

Fetch

Google

Target

SourceLevel - PRs Metrics and automated Code Review

2022 - HackerNoon Contributor of the Year - Golang

SourceLevel · All-in-one Data & Analytics for Engineering Teams

Newsletter

Book a Call

Nominated for 2022 - HackerNoon Contributor of the Year - Golang

CTO at SourceLevel.io

Too Long; Didn't Read

In your car, at home, or at work — Bosch technology shapes many areas of life.

Pull Request Checklist: What You Need to Do Before Assigning a PR to Someone

Too Long; Didn't Read

Companies Mentioned

Weverton Timoteo

About Author

TOPICS

THIS ARTICLE WAS FEATURED IN...

Pull Request Checklist: What You Need to Do Before Assigning a PR to Someone

Too Long; Didn't Read

Companies Mentioned

Weverton Timoteo

About Author

TOPICS

THIS ARTICLE WAS FEATURED IN...

RELATED STORIES