3

Suppose you're getting the latest version of a file from source control and a conflict occurs. There are four possible types of changes in the conflict:

  1. Yours
  2. Theirs
  3. Both
  4. Conflicting

Naturally, in cases 3 and 4 you'd want to review the changes manually (in the latter case you even have to resolve it). Regarding cases 1 and 2 I'm not sure, but I'm thinking only changes of type 2 (Theirs) should prompt manual inspection. The reason being, you know what you changed, as opposed to "their" changes which may break your changes. So I'm thinking one could skip reviewing his own changes (case 1), and manually review / resolve the other changes (cases 2-4).

Other developers suggested more extreme approaches - such as only reviewing conflicting (or "both") changes and trusting automerge for the rest. The other extreme is reviewing all the changes every time.

Of course, other parameters may factor in. For example, if you only made a single correction to a file, and there are 200 changes made by others, then provided you can be reasonably sure that your change is unrelated (say it's an SQL file and you wrote a new separate stored procedure), perhaps you can skip reviewing the other changes.

I am torn between the dangers of not inspecting changes and the time and energy that could be saved by trusting the automatic merge. Supposedly, you have compilation and unit/component/e2e tests that will expose bad merges.

Improve this question
2
  • Are you using Perforce? I've never had any issues with bad merges with Perforce. However, git on the other hand...
    – James
    May 29, 2013 at 19:02
  • We're using Source Depot, which is Microsoft's fork of Perforce, but I think that's irrelevant. The merge logic itself lies with the merge tool used, in my case KDiff3.
    – Ohad Schneider
    May 29, 2013 at 21:16

4 Answers 4

Reset to default
5

Supposedly, you have compilation and unit/component/e2e tests that will expose bad merges.

This is the important part to me. In general, I will trust the auto-merge for code files (but not markup, project files, etc) where it can actually run (not conflicts). It tends to mangle projects and markup, but code files are (usually) fine.

But I also work in a place with frequent commits, good unit test coverage and gated check-ins (if the build/tests don't succeed, the commit does not work). Oh, and my release date is months into the future, not hours... 95+% of the time, it works just fine and I save the time and effort of looking through the merges. Different environments call for different carefulness.

Improve this answer
3
  • Well, I work for Microsoft and I'd like to think our environment is pretty good :) So you're saying you'll "completely" trust automerge for code files?
    – Ohad Schneider
    May 29, 2013 at 13:24
  • 1
    @ohadsc - No, but I am lazy and trust it enough that using it universally will save me more time than I need to spend fixing its occasional screw ups (in this environment).
    – Telastyn
    May 29, 2013 at 13:35
  • Yes, that was along my line of thinking as well. Maybe I'll give this approach a try in my environment, I suspect that I'll come to the same conclusion as you did.
    – Ohad Schneider
    May 29, 2013 at 13:44
2

Even manual merging can be quite dangerous. Reason is that even if you do manage to merge it so that the code is compilable, the behavior of the code might be different from both original and your version. Martin Fowler has bliki explaining more into details.

And this is also reason you should integrate often with main repository. So you don't have to resolve crazy conflicts that encompass dozens of files.

Improve this answer
0
2

You can trust automatic merges if -- and this is a big if -- you have everything else in order. This means that you and anyone else working on the same file are aware of what's going on and have sanity-checked all of it before diving in.

Per Cederqvist said it best in section 1.2 of Version Management with CVS, which I make version control noobs read whether we're using CVS or something else:

CVS is not a substitute for developer communication.

When faced with conflicts within a single file, most developers manage to resolve them without too much effort. But a more general definition of "conflict" includes problems too difficult to solve without communication between developers.

CVS cannot determine when simultaneous changes within a single file, or across a whole collection of files, will logically conflict with one another. Its concept of a conflict is purely textual, arising when two changes to the same base file are near enough to spook the merge (i.e. diff3) command.

CVS does not claim to help at all in figuring out non-textual or distributed conflicts in program logic.

Improve this answer
5
  • I am definitely NOT in sync with everyone working on the same files as I am. Some of them reside in other countries. Most times I'll have to divine their intentions (to a certain degree) from the code.
    – Ohad Schneider
    May 29, 2013 at 14:24
  • @ohadsc: Then I'd say you need to fix that problem first. Getting your ducks in a row ahead of time saves a lot of time and aggravation.
    – Blrfl
    May 29, 2013 at 14:27
  • Not sure if that's realistic. There are too many groups, too many developers (from different places), too many features. I can't possibly know what they are all doing at any given time, and the codebase is joint. The only plausible approach I can think of is maybe contact whoever made changes to a certain file if his code changes aren't clear to me. A little problematic, especially due to time zone differences, but probably doable.
    – Ohad Schneider
    May 29, 2013 at 14:39
  • @ohadsc: Sounds to me like you're in an environment where random people dive in and change the code as they see fit and there aren't any tools in use (e.g., an issue tracker) to document why changes were made. In that environment, whether or not you trust your VCS to do auto-merge is the least of your problems.
    – Blrfl
    May 29, 2013 at 21:57
  • Far from it! We have a very organized issue tracker in TFS, detailed changelist information, thorough code reviews on each commit, comprehensive tests at each level (UT, CT, E2E), continuous integration, gated check-in, etc. I see how my comment could have been interpreted like this though, but I simply meant there are many developers and it would be hard to keep track of what everyone is doing. However, perhaps it just looks like that to me now because I'm new, and perhaps more communication is something I need regardless.
    – Ohad Schneider
    May 30, 2013 at 8:42
1

If you have many instances where there are 200 changes to a single file on a merge, I don't think you are merging often enough, and it's very likely that there is a complexity problem with that file.

We have bad merges probably once every few months, that we know of. Without some sort of release engineering team to investigate every change before a new release is created, or very good peer review, you aren't going to be able to catch every bad merge - if you have a bad merge in your unit tests and a bad merge in your code, which is more likely than one would think, then you wont catch the problem until something goes very wrong (or someone, probably not in IT, notices catastrophic data loss).

If you don't put any effort in, you can be 99% sure that everything merged fine. If you're willing to put in an extra 30-40% of release and change management time, you can probably be much closer to 99.99% sure. For a company like Microsoft, it's probably worth the investment, but for many others it isn't.

Improve this answer
1
  • 200 changes was an extreme example but your points are well taken.
    – Ohad Schneider
    May 29, 2013 at 14:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.