Is there justification for leaving conflict markers in checked-in code?

Question

Consider conflict markers. i.e.:

<<<<<<< branch
blah blah this
=======
blah blah that
>>>>>>> HEAD

In the particular case which has motivated me to post this question, the team member responsible had just completed a merge from upstream to our branch, and had in some cases left these in, as comments, as a sort of documentation over what had just been resolved. He left it in a compiled state, tests passing, so it's not as bad as you would think.

Instinctively though, I really took objection to this, however being devils advocate to myself I can see why he might have done it:

• because it highlights to other team developers what has changed as a result of the merge.
• because those who are more expert with the particular pieces of code can then address the concerns illustrated by the comments so that he doesn't have to guess.
• because the upstream merge is a right pain and it can be difficult to justify the time to resolve everything well and completely, so some semi-complete FIXME notice is necessary, so why not use the original conflict as a comment to document this.

My objection was instinctive, but I'd like to be able to justify it rationally, or see my position more wisely. Can anyone give me some examples or even experiences where people have had a bad time with someone else doing this and/or reasons why it's bad practice (or you can play devil's advocate and support it).

My own immediate concern was that it would clearly have been annoying if I had been editing one of the files concerned, pulled the changes, got real conflicts, but also pulled in the commented ones. Then I would have had a very messy file indeed. Fortunately that didn't happen.

Answer 1

This is clearly wrong. It is the job of the version control system to keep track of changes, and it is the job of diff tools to show what has changed as a result of the merge. There should be a comment in the commit log, and maybe in the code, explaining what was changed and why. However, IMHO, leaving the conflict markers in as comments is the same as leaving dead code around.

Answer 2

I've had similar problem with some code either being commented out (which is somehow simillar to your case) or moved to a method not being actually called anywhere. When asked why people do this the response was that they feel a bit safer when they have some code block still around. The most obvious counter argument is that it is VCS job and not theirs. However, there is also another aspect. When somebody else is reading code while learning or making changes, he will probably be side-tracked by such a comment. He will definitely read it and perhaps spend some time to understand why it is here and what possible correlation it has to his current job. As a conflict marker is a sign of a conflict, that already have been resolved, this is for sure a waste of time.

Answer 3

I think comments should refer to the code that's there, not to code that has been there in the past, nor to events that happened to the code sometime in the past, nor to code that existed in a parallel universe (another branch) in the past. Leaving the markers in the way your team member did creates at least three problems:

1. The original code probably was something like blah blah null, and the bug report said "Can't use null there, use this or that, or whatever." So two people independently fixed the bug and when the fixes were merged, the conflict arose. Now the comment documents not what the problem was nor what the fix fixed, but only that there were two different fixes at some point in the past. That's not very helpful. A comment like //blah blah needs a non-null argument would at least give an indication what changed (and even that information is more easily available from the version control system's commit comment).
2. The merged version might not even look like one of the original lines. Maybe if you want blah blah to take this and that, the correct form is blah blah (this,that) or even something more complicated. In that case, leaving the conflict message in as a comment will surely confuse anybody trying to read the code later on.
3. Most version control systems give you access to the project history. For example, i can right click on a file in eclipse (with svn), say "Show History..." and then say "Compare Current with...' and get a diff window that highlights the differences. That information is way easier to grok if the diff highlights contain the actual differences, and not the comments around them. Every bit of non-functional change in the code makes that diff harder to read.

Answer 4

How annoying are conflict markers in checked-in code?

So annoying.