Your Build Smells: Avoiding Painful Merges


23 December 2015, by

This post is the first post in a series looking at Build Smells – signs of major issues in team build processes – and the things we can do to try and solve them. Watch this space, or follow us on twitter to see more of these as we publish them.

Painful Merges

If you’re not careful, it’s easy to end up in a situation on a project where you end up consistently wasting time fighting merge conflicts, rather than getting useful things done. This isn’t unavoidable, and it’s a build smell: a sign that all is not quite right here.

We see cases where this is not just occasional, but actually becomes completely standard, and there’s simply an accepted expectation that teams working together will burn time every couple of weeks, as somebody spends a day merging everybody’s changes together (or a week a month, or worse). It’s not uncommon to see cases where this is even encouraged: where everybody in the team automatically starts a branch every sprint, and everybody merges them all together all at once at the end (nearly two weeks later). This is a recipe for pain.

This wastes time and saps morale. It’s even worse than that in reality though, because if you’re continually expecting to hit merge conflicts then you can’t really refactor; any wide-ranging changes to the codebase either require everybody to down tools, or to risk huge time-wasting conflicts. You end up in a situation where your build processes and tools are pressuring you not to refactor, where your own build is encouraging you to write worse code. Renaming a method becomes a painful and potentially costly process, so you don’t.

This is a build smell. Merges should be frequent, small, and typically easy. If you see people burning large amounts of time merging, actively avoiding merging their code, and racing each other to push changes to avoid having to deal with the repercussions, you’re seeing this in action.

How do these happen?

We can examine this more closely at this by looking at some of the most common forms of merge conflict:

  • Textual conflicts

    Conflicts where two changes have been made that are totally independent, but which don’t have a clear resolution when merging textually.

    For example: two people adding two new fields to a class in Java, on the same line. The fact they chose the same line typically makes no difference, and the fields typically independent, but all major merge tools will ask you to resolve this manually, as they have no way to decide which line should appear above the other, or whether that can be done safely.

    Interestingly, this conflict transparently disappears if we make these changes in sequence. If one person adds a field, and pushes the code, and another pulls the code, and adds a field, then there are no merge conflicts, and the situation is invisibility resolved with zero extra effort from either party.

  • Simple semantic conflicts

    Conflicts where two changes have some interaction that produces incorrect behaviour, but aren’t necessarily in direct opposition. Changing a getDistance() method to return a distance in meters instead of centimeters for example, while somebody else adds some code that calls that method.

    These changes may well not textually conflict, but do have a relationship: code that’s expecting centimeters will (probably) not act correctly when given a different unit.

    (As an aside: issues like this are a good reason to include units of measure in your types, to use things like F#’s built-in unit of measure support where possible in languages that support that, or to at least use clearer method names).

    Again though these conflicts are trivial to resolve if the changes are made in sequence. If you are already aware that the method returns meters, because that change has already been shared, then you’ll ‘resolve’ the conflict without even noticing, by writing your code with consideration of that built-in from the start.

  • Fundamental semantic conflicts

    These are the conflicts where you and somebody else are trying to make changes that are fundamentally in opposition.

    Perhaps I need a button to be green, and you need it to be red, or I need to make a major refactoring of a class that makes some functionality impossible, and you’re writing some new code that depends fundamentally on that functionality existing.

    These are conflicts without easy resolutions. To solve this, we’re going to have to have a discussion, and come to some compromise in our goals and overall approach.

    These issues are also far rarer than people imagine though. Try to think of a conflict recently that would’ve been an issue if you’d made your changes entirely in sequence, rather than in parallel. It’s rare that two developers on the team are chasing mutually exclusive goals at the same time.

    Notably when this does happen it’s still much easier if we can resolve these immediately. If I find out that we can’t make buttons red when I make the first button red, I’ve got a problem to solve. If I find out that we can’t make it red after I’ve spent three days redesigning the whole application in shades of burgundy then I’ve wasted a huge amount of time, I have to undo all that, and I still have the same problem to solve too.

When we hit painful merges, it’s rarely because we’ve actually hit fundamental conflicts. Most of the time, we’ve just built up a large set of textual and relatively simple semantic conflicts all together, and they’ve snowballed into a huge conglomerated mess that we now have to unravel.

The underlying conflicts here come from isolation. Code that’s kept on a branch or kept locally unpushed is code that’s isolated from everybody else on the team. They’re secret changes you’ve made, that you’re not sharing with everybody else. Because you haven’t shared them, nobody can take them into account in their own changes, and conflicts quickly proliferate.

This gets dramatically worse as isolation increases. If you keep code on a branch for a day, you have one day of your changes to potentially combine with one day of everybody else’s on the team. If you all stay on your branches for a week, you’ve got a whole week’s worth of your own changes to combine, with every other person’s week-sized chunk of work too. This quickly gets out of hand.

What can we do about it?

The solution here comes neatly out of the above: large conflicts are typically a collection of many relatively simple conflicts, and each of those would almost disappear if the changes were made in sequence. Therefore, if we want to get rid of merge conflicts, we need to move as much possible away from isolating our codebases, and get as close to being a team of people making small changes in sequence as we can.

To fix this smell, share your code early. As early as possible; I’d suggest you integrate your code with the rest of your team every time you have code that doesn’t break anything. As soon as you have code that compiles, passes your tests, and functionally works (to a level where you’d be happy showing it to a customer): share it with your team. Sharing this won’t break anything (by definition), and completely eliminates any risk of conflicts within that code.

That’s all well and good, but much easier said than done. Let’s look at some examples:

  • Sharing tests

    If you’ve written a couple of new passing tests in a clean codebase (for example, just before you’re about to refactor something), you can commit them and share them immediately. They’re passing tests, so you can be pretty confident you’re not going to break anything, and once pushed you remove any risk that they’ll conflict with others in the team.

    If you don’t do this for a while, you build up isolated changes. When somebody attempts to refactor test code in the meantime, touch any of the dependencies you’re using, or actually touch the code you’re testing, they have no way to easily take your changes into consideration.

    Meanwhile if you have pushed your tests then it’s easy for others to check that their changes don’t break it, or to apply their wider refactorings to your test too (often automatically, with automatically applied refactorings).

    Doing TDD, and you’ve written a failing test instead? There’s reasonably mileage to be gained in many cases in ignoring the test (e.g. with @ignore in JUnit), and sharing that (although this does require discipline to ensure you don’t leave tests ignored long term).

    You don’t gain any guarantees in this case that the behaviour under test won’t change, but you do ensure your code will continue to compile, and others making wider changes that affect your code will fix it in the process of those changes like any other code in the codebase. By sharing your code early here, you allow people making other changes to do so with respect to yours immediately, rather than leaving you to tidy up the mess later.

  • Sharing model and component changes

    If you’ve just added a field, or a new method to the models or internal components of your system, you’re almost certainly very quickly back in a functionally working state where your changes could be shared. A remarkable amount of the code you add as you progress towards building new functionality doesn’t break your application for your users or other developers, and can be easily shared immediately.

    Added a new calculateCost() method to your Invoice class as the first step in the feature you’re building? Share that now, and the other developer who needs to refactor Invoice tomorrow for their changes won’t step on your toes, and you’ll both live happier lives.

    This is especially relevant in wider-ranging cross-cutting changes. If you’ve refactored big chunks of code, changing how logging works, or renamed some fields, it’s should be very unlikely you’ve made any user-facing changes to your application at all, and your tests should be catching any such issues if you have. In addition, the stakes are higher; refactorings particularly quickly collect conflicts, while the majority of them won’t have any fundamental conflicts at all (rename field/variable, extract method/class/interface, etc), and fundamental refactoring conflicts that do appear will explode far more painfully if left to ferment for a few weeks.

    Sharing changes like early drastically reduces the potential chance and damage of conflicts, and is well worth pushing for where possible.

  • Sharing trickier changes

    These are two examples where it’s particularly easy, but they don’t cover every case. Often you want to make a change that is fairly large in itself, and individual changes within that will result in broken or incomplete functionality. Rewriting the interface for a page of your application is a good example, performing a huge refactoring or rewriting a major hefty algorithm within your application; in many cases this might be a large and challenging change, and nobody wants to share a broken or incomplete page or algorithm with the rest of their team (let alone their users).

    There are things we can do in this case however. Using feature flags/toggles and branching by abstraction are two particular approaches. Both revolve around sharing code changes immediately, but isolating the user-facing functionality within the codebase (rather than isolating the functionality and the code changes, using version control).

    Branch by abstraction works by introducing an abstraction layer above the code you’re going to change, which delegates to either the old or new implementations. Initially this calls the old implementation (changing nothing), but once standalone parts of the new implementation are completed and ready the abstraction can be changed to call through to new code instead, and eventually the old implementation (and potentially the abstraction) can be removed.

    Feature flags work somewhat similarly, but dynamically with configuration, either application-wide, or in per-user settings. A conditional statement is added at a relevant branching point for your change, switching between the two implementations based on the current configuration (return flags.newUserPageEnabled ? renderNewUserPage() : renderOldUserPage()).

    This has a few advantages, notably making local development of visible features like new pages easier (just enable the config property only on your machine), and allowing you to take this even further to become a feature in itself (as done by Github, Flickr and Travis CI) providing you fine-grained control over your feature release process, for progressive rollouts or A/B testing.

All of these techniques help you share code earlier, and sharing code early enough can almost totally remove conflicts from your team’s process, and make them easier to deal with when they do happen.

I should be clear that I’m not saying branching is always a bad idea. Branching has many sensible practical and effective uses, and it’s an incredibly powerful tool. Unfortunately many teams fall back to it as their goto tool for every change they make, and they start to build the way they work around actively keeping things on branches.

Branching comes with a cost, that rapidly increases over the lifetime of the branch. That’s fine if you’ve got a specific benefit you want to get from branches, but not a good idea if you don’t. Some good reasons to use branches for example: very experimental code that’s unlikely to make it into production, situations where you can’t allow everybody push access to master directly (as in open-source and some regulatory environments), and situations where your review processes don’t work effectively with it (on of the main reasons I see for not following this, as Crucible is one of few good tools that will easily let you review commits on master).

Nonetheless, in each of these cases it’s still worth aiming to merge more frequently. Encourage many small pull requests to add a feature, don’t allow experiments to go on too long with deciding whether they’re worth committing to, and push for more smaller reviews with minimal turnaround time before merging.

In general, smaller and more frequently shared changes disproportionately reduce the risk of each change, make refactoring vastly easier, and can often make conflicts disappear with no extra effort. A large proportion of merge conflicts simply are simply invisible if the changes are made in sequence instead, and sharing code earlier moves you closer and closer to this situation.

Finally, it’s notable that this way of working isn’t just a benefit for conflict resolution. It is a good idea all the time to aim to break your work into many small chunks, with many steps at which the code is fully working. Doing so helps avoid yak shaving, makes each of your changes easier to understand, and has lots of benefits for time management and estimation too. There’s a lot of upsides.

Caveats

Hopefully that’s an interesting insight into the kind of issue we see in projects, and some of our approaches to improving them!

We do encourage all our teams to find their own solutions independently for their problems however, and I’d be lying if I claimed this is our standard go-to approach in every case. It is an approach that many of us have found effective in places though, and it’s well worth considering as an option if you’re facing these same challenges in your team.

I should be clear also that I’m not suggesting that branches are never useful, that this is the One True Way of managing version control, or that you should dive into this 100% immediately. If your team is facing concrete issues with conflicts and merges, experiment with reducing isolation and sharing code earlier and more frequently, and measure and discuss the changes you see in practice. We’ve found benefits in many cases, but that is by no means definitive, and there will be cases where this is not the right choice.

If you’re interested in more thoughts on this topic, I do recommend the writings of Martin Fowler, particularly around feature branches, and Jez Humble and Dave Farley’s book ‘Continuous Delivery‘.

Let us know your thoughts all this below, or on twitter! In the next post, coming soon, we’ll take a look at the next smell in the series: Nervous Pushes.

Tags: , , ,

Categories: Technical

«
»

Leave a Reply

* Mandatory fields


seven × 8 =

Submit Comment