What makes the perfect pull request if one should ask?
The answer to this will vary according to set standards in a team. Code review is a huge factor in ensuring quality codes are used for building a software, it also allow team members to have an insight on what the next guy is working on. Key-man risk is not a good thing to have in a software development team, frequent PR is a great way to keep everything in check.
Over the years working with various teams, I have come up with my top "must-do" for a pull request.
The Build-up
Pull Requests consist of code changes (addition, modification or removal) which have been built over a short period.
1. Code Commit to Branch
All pull requests starts from a commit to the local branch. These commits are more important than many engineers may think. The title of commits should be descriptive enough to tell what was done/committed into branch. This ease rollback or branch-out in case of issues that require you picking your last working commit.
PR titles like 'first change', 'second fix', 'commit before PR', 'working code' really do not cut it.
2. Size of Changes
A Pull Request is best reviewed when the changes focus on small chunk of codes and are related. Bundling 2k line of changes across 63 files is a sure way to say "hey dude, just approve, the code works". Different scenarios may prompt this such as working on an off-the-chart feature or engineers to review may be on work leave/vacation at the time, or a poor code review culture in a team.
3. Descriptive Naming and Summary
Naming is a problem for many developers (maybe only the one's I've worked with). From poor commit messages to terrible PR titles such as 'minor changes', 'fix', 'small update', 'add log', 'working code'. PR titles should be able to describe the purpose of code being checked in as it provide insights just like commit messages. The description of should also be filtered properly to give clear insight to the code reviewer. Below is a good sample PR naming.
The above is short and precise. It's really bad finding out the purpose of the PR after looking at the code with a title 'minor update'.
4. Link Related Work-items
In platforms like Azure DevOps that support work-items for tasks, user-stories and bugs, it's best to create these items that describe the scope of work to be done. It is also a link to keep reviewers updated with what the actual logic should do. Though reviewers barely open linked items to review according to how the code is written, it is still a good practice to "keep the code with the requirement".
5. Pay Attention
Yea! Pay attention to the destination branch, especially in cases where build pipelines are setup for similar branches like DEV and MASTER. This could be a skip for code reviewers as well, you may ask how?, if it's possible, then it's possible. Attention is required by the creator and reviewers of the PR to make sure codes are well scanned, the destination branch is the intended one as well.
6. Review Your Own Pull Request
I have made it a habit of always reviewing my own PR even before I finalize it. When done finessing the title and description, just switch to the 'File Changes' tab and review every line once again. This is the point where I make follow-up commits for 'cleanup' as sometimes I see some unwanted commented code, a debug log, typos and more. Be your no. 1 critic, this way you won't get cheap points scored on you for a minor under-sight.
EOL: End of Line
This is actually conclusion.
Maintaining a set standard in a team requires discipline, code reviews is such one that requires the team to enforce a standard and not shy away from making changes as new patterns are discovered. Do raise the red-flag when an anti-standard PR is seen. The whole essence is to ensure quality code and business continuity seamlessly. Sometimes, I do err in code review - just keep the percentage of good on the rise until it becomes a habit.
-:)
Cover image source: oroinc.com
Top comments (0)