After submitting code for review, you will often receive requests to include fixes or improvements not directly related to the primary goal of your change. You can recognize them easily as they usually sound like: “Because you are touching this code, could you…?” and typically fall into one of the following categories:
- Random bug fixes
- Code refactoring
- Fixing typos in code you didn’t touch
- Design changes
- Increasing test coverage in unrelated tests
While most of these requests are made in good faith and aim to improve the codebase, I strongly recommend against incorporating them into your current change for a couple of reasons:
You will trade an important change for a lower-priority one
Your main change was the reason why you opened the pull request. By agreeing to additional, unrelated changes, you sacrifice the more important change for minor improvements. How? The additional changes you agreed to will need at least one more round of code reviews, which can trigger further feedback and more iterations that will delay merging the main change.
I learned this lesson the hard way when I nearly missed an important deadline after agreeing to fix a ‘simple bug’ that was unrelated to the feature I was working on. Fixing the bug turned out much harder than initially perceived and caused unexpected test issues that took a lot of time to sort out. Just when I thought I was finished, I received more requests and suggestions. By the time I merged my pull request, I realized how close I had come to not delivering what I promised on time, only because I agreed to focus on an edge case that ultimately had minimal impact on our product.
You will dilute the purpose of the pull request
Ideally, each pull request should serve a single purpose. If it’s about refactoring, only include refactoring. If it’s fixing a bug, just fix the bug. This approach makes everyone’s job easier. It makes pull requests easier and quicker to review. Commits are less confusing when looked at months later and if you need to revert them, no other functionality is lost. All these benefits diminish if your pull request becomes a mixed bag of random code changes.
How to deal with incidental requests?
You have a few ways to proceed with suggestions you agree with. Usually, the best approach is to propose addressing them in separate pull requests. If your plate is full, log tasks or add them to the project’s backlog to tackle them later. Occasionally, a request may suggest a significant redesign that may fundamentally alter your work. If the idea resonates with you and won’t jeopardize your timelines, you might want to consider implementing it first and redo your changes on top of it.
Top comments (0)