It is easy for a developer to think about their code changes as the end product of their work. Often the thought is that if the pull request contains the needed changes then not much else matters. However accurately communicating and recording the intent of those changes is crucial. Proper communication of intent saves time, helps to bring misunderstandings to the surface, and promotes a growing understanding of the code base. Maintaining a detailed record of not only the changes but also the intent of the changes helps secure the future of the code base and can act as a form of organic documentation to those who come later. This is achieved by not simply opening pull requests but crafting perfect pull requests.
You start by looking at commits. You want to break up commits by type and intent. This forces small commits that can act as easily understandable steps towards a solution. Here is a list of the the types of changes that commits can be broken into:
- Code formatting
- Refactoring
- Bug fixes where the end behavior does not change
- Backwards compatible feature changes
- Backwards incompatible feature changes
Breaking up commits into these categories will help provide a clear focus to each change and prevent refactoring from being misunderstood as a feature change or a bug fix as being misconstrued as a new feature. This will provide the ability for clear and descriptive commit messages each of which can be centered around a single intent. If a commit message is long and addresses multiple topics then you might need to separate out the changes into different commits in order to maintain the single intent principle. This will naturally make for small commits that are readily consumable by someone who needs to step through your problem solving process and who might be unfamiliar with the background of the change.
Besides the code itself the most important aspect of a pull request is not only a description of what changed but an explanation of why the change was needed. Links to a ticketing system can often serve this purpose, prevent duplicate information, save reviewers time, and serve as documentation in the future. This also provides the reviewer context so that they can provide a purposeful code review.
Additionally pull requests often need test data or test scenarios in order run through the changes in a cohesive way. The submitter should provide this information along with the pull request itself. This will encourage the reviewer to pull these changes into their local environment. Links to development and production environments of where the changes are happening can provide context, save time, and provide useful documentation of what the change is and how to see the change in a live environment.
Careful attention should be paid to whether a comment should be in the pull request or in the code. Counter intuitive code should be explained in a code comment for others to see instead of being explained in the pull request. The pull request would be a good place to explain why no alternative solution is available and might save on back and forth communication.
Intuitive code that seems wrong for one reason or another but is the only way to solve the problem is likely a good candidate for a comment in the pull request. This will short circuit back and forth conversation with the reviewer but is likely not something that should live on in a code comment. Often it is also good to invite the reviewer to provide alternative solutions in these scenarios.
Sometimes there are multiple equally valid solutions. In this case the submitter should explain the decision they made and again invite the review to suggest an alternative. Asking for a reason for the alternative solution can help align development practices in the long term.
Finally you might need to add next steps to the pull request. Should this pull request follow the normal development process? Does it need added to a hotfix? Does it need escalated so that someone else can make that decision? Do content or database updates need to go along with this change? Do deployment steps need taken after this change is live? Answers to relevant questions such as these should be provided with the pull request. This then can be referenced as the changes propagate through different environments or discussion occurs in reference to the changes.
I have covered many aspects of pull requests and not all of them apply all of the time. Often development practices will dictate modifications to these guidelines. However having these things in mind when “crafting” a pull request can help save time, promote good code, increase knowledge sharing, and create lasting documentation.
Leave a Reply