Small Pull Requests And Version Control

I am a big fan of having small pull requests, but this is not always easy to achieve. I have worked in many organisations where PRs of hundreds of lines of code is the normal situation, because of multiple reasons. In this blog post, I want to go into detail about why it can be hard to have small pull requests, what you can do about it and how it will affect your version control system.

In many organisations it is the rule that a single ticket or issue must be delivered by a single pull request. In some organisations it is even the rule that the pull request must be squashed into a single commit. The reason for this is mainly clarity, you know exactly which one ticket is responsible for implementing a feature or fixing a bug. This has however the result that the single PR may be rather large. There are of course a couple of ways to prevent this if you want to have smaller pull requests:

A first simple solution is to split up the ticket. Smaller tickets will automatically result in smaller pull requests. There are many good reasons to have small tickets (besides from having small pull requests), but you can only split up a ticket so much before the split becomes a technical reason instead of a functional reason. I typically prefer to keep my tickets focussed on user functionality, describing them as stories from the point of view from a user. For instance, a ticket to add a new object the user can create, modify and delete can be split up in the create, modify and delete parts. But each ticket (the first one more so) still has to implement quite a lot of code. You have all of your entities, services, controller and repositories as well as the UI. Splitting the ticket up any further would result in a ticket that says to create a database table for the new object, and is no longer user oriented. But is there a reason why I can't first merge the database schema change with maybe just the java DAOs and the repository?

Another approach is to have a single feature branch and branch off of that feature branch with separate smaller branches. This way you could create a pull request from your sub-branch into the feature branch. This allows for small pull requests and in the end you still end up with only a single commit/PR into master. For me, this isn't really a solution, as this either means that you won't properly review the final PR into master, which is the most essential one, or you still have to review the large PR even though you already reviewed all of the code resulting in more work/overhead for the development team.

The bigger question however is why a single ticket should be a single pull request? Are you afraid of not being able to release/deploy the code because of a half implemented piece of code? Test Driven Development (TDD) should make sure that all code is properly tested and thus you won't ship any broken code, when I say you should create small PRs, I don't mean you should create a PR for something that is only half-implemented. Each PR should be fully functional (as guaranteed by TDD). The functional part is however still a concern, maybe you don't want the user to already be able to add the new object type since it can't be modified or deleted yet. In that case you either don't show it in the UI yet, or you can use feature flags to disable it (or both).

The reason why you should have small PRs is because it is proven that smaller pull requests result in more defects being found. They can also be reviewed faster and thus you can merge and release faster. Nothing is more annoying than being blocked to released because the pull request isn't reviewed yet.

What about the impact on your version control system? It is true that it makes reverting a version more difficult since a feature is spread out over multiple commits which may be intertwined with other features. If you however release fast, this may not be a problem, as typically reverting to the previous version should suffice, and if not it may also be faster to simply implement a fix rather than deploying an older version. Releasing faster also typically means that any bugs that are discovered are rather small and limited to a single location.

The history in your VCS may also becomes less 'readable' since it becomes a mess of many commits all doing different things. I never found this to be an issue since most organisations already adopted a policy to mention the ticket number in the commit message. This means that it is rather easy to search for all tickets involved in a ticket.

If you are still not convinced about multiple PRs and merges per ticket, you may just want to start with creating small commits locally for small changes. The final PR will still contain all of these commits, so the size will still be as large, but the reviewer can review changes commit per commit, which tends to make it easier to understand your reasoning and follow the flow better. Once reviewed, you can still decided to squash the commits into a single one and merge. Note that this is not my preferred approach, but it may be a safe way to experiment with the idea of creating smaller commits/pull requests. It may serve as a first step to a more radical approach.