You've done the hard yards, that new feature or bug fix now works just as you'd like - now all that is left is to submit your Pull Request, get it landed and everyone is in profit, right?
Not so fast.
It's two, three, four days later and you're still battling to land that Pull Request (PR) as you deal with suggestions to refactor your code, Quality Assurance (QA) bugs and merge conflicts. š
The feeling of accomplishment you had initially when proposing your change is now a feeling of resentment as you're forced to revisit your code again and again. š¤¬
So, how can we avoid this scenario and ensure that your Pull Requests are landed quicker and with less hassle?
-
Limit the scope of your change
You should always try to keep your pull requests as small as possible. Focus on fixing or adding one thing at a time. The more changes you include in a PR, the higher the risk that you introduce bugs.
-
Avoid drive-bys
A drive-by is a change or refactor you make while browsing through the code and you notice something obvious that should be changed. It might be as simple as a missing
alt\tag on an image or updating the indentation of a code block. By all means, make these changes, but propose them separately. -
Rebase before proposing your changes
Rebasing your changes on top of the branch you're proposing your changes to will reduce the chance of merge conflicts and also mean you're proposing your changes against the codebase as it exists in that moment, not as it was when you created your feature branch.
-
Make sure you run the tests locally
If the project has a test suite, which hopefully it has, be sure to run those tests locally before proposing your changes. This should help highlight errors or linting issues in your changes.
-
Siphon off non-blocking issues
Often when a person is reviewing your code, they will offer suggestions that are very valid but not strictly show-stoppers for the change you're proposing. These might be UI improvements, code abstractions or refactors to existing features. In my opinion, so long as your change does not exacerbate an existing issue, these recommendations should be syphoned off as separate issues to be prioritised accordingly.
Incorporating the above steps into my own work has really increased the cadence at which I get my PRs merged. Hopefully, it might help you too. š„
Top comments (1)
Great list! I think breaking things into other PR's and/or Issues is super important to a good PR process
I especially agree with #1, and wanted to add one more reason to keep each PR as small as possible. And that is to aid the people reviewing your code! Not only is there less likely hood of bugs, but it also just makes it easier to review!
You'll have been working on this change for awhile, and it feels familiar to you since you wrote it. But, unfortunately, your reviewers are coming in a bit blind and won't be able to easily understand as big of a solution right away. So the smaller and more focused you can keep your PRs the easier the process can and will be for everyone!