[MERGE] Teach 'bzr remerge' to not use 'iter_conflicts'

[John Arbash Meinel]

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>>> By the way, I'm going to require that we resolve this in one way or the
>>> other for 0.12. So if you aren't happy with my proposed solution to
>>> filter the conflict list, then I can revert the change, and we need to
>>> undeprecate iter_conflicts().
> 
> I am glad that you want to solve this problem now.  Yet I am also
> frustrated, because I have tried to get people to discuss this problem
> in the past, and basically been ignored.

I'm sorry if I have contributed to you feeling ignored. I think it is
one of the dynamics about having a fairly high development rate, versus
the distance of the people involved in the project. It is pretty easy
for things to get lost with all of the emails that get generated. Given
everything, I think we actually do quite well. But there are certainly
aspects that can be improved.

I hoping that now we have switched to a 1-month release cycle, along
with release managers that sort of see each month progress. We can get a
better idea of where the project is headed in the short term, and it
gives people a weekly place to put reminders about things that may have
gotten missed.

I certainly feel like some of my changes get missed (I have at least 4
things that didn't make it into 0.12 because nobody reviewed them, and
I've been sitting next to the people who should have done it.) A lot of
it sat for more than a week without getting reviewed. And I even
included it in my weekly summary. So stuff does get missed. I think
there have been a couple proposals, at least for having a review queue,
that could help to not miss code.

Also, I think the release manager for a given month could probably do
some of the delegation of responsibility, asking people to specifically
look at a patch. I tried to do that a little, and I think it could be
done even more in future releases.

> 
>>> I personally feel like iter_conflicts() is trying to read too much into
>>>  filenames, but it is your code. 
> 
> I am happy to solve it in terms of the conflict file mechanism for
> WorkingTree3.  Doing it that way for WorkingTree2 is double-handling,
> but maybe we don't care enough to fix that.

True, but it is a small overhead in WorkingTree2, and keeps the codebase
simpler for more current formats.

> 
> One of my hesitations has been the Python aversion to checking type;
> calling isinstance(conflict, ContentsConflict) seemed uncouth, but I
> wasn't sure what The Right Way was.
> 
>>> So if you feel strongly that remerge
>>> needs to use iter_conflicts(), then I can revert my changes, and
>>> undeprecate iter_conflicts().
> 
> Alternatively, we could create iter_contents_conflict.
> 
>>> It would be nice to get a few more test cases to indicate *why* we need
>>> to do this, so we don't break things in a future refactoring.
> 
> Sure.
> 
>>> We can filter based on whether it is a content conflict, but using
>>> 'iter_conflicts' will also cause us to flag real versioned files that
>>> happen to end with .BASE, .THIS, or .OTHER. Which is also a bug, though
>>> perhaps less obvious.
> 
> I have no problem with using the conflicts file for WorkingTree3.  But
> there's no reason WorkingTree3.iter_conflicts couldn't do that.
> 
>>> # Remerge only supports resolving contents conflicts
>>> allowed_conflicts = ('text conflict', 'contents conflict')
>>> restore_files = [c.path for c in conflicts
>>>                  if c.typestring in allowed_conflicts]
>>>
>>> This passes the test suite, and should satisfy your 'only allow text or
>>> content conflicts'.
> 
> This seems like a fine approach.

I went ahead and submitted this change, since you seem okay with it.

> 
>>> You claimed remerge only supports 'content conflict', but when I did not
>>> include 'text conflict' the test suite didn't pass. And I saw that old
>>> working tree formats used to return both of these types.
> 
> Sorry.  I consider text conflicts to be a form of contents conflicts,
> and I thought that was reflected in the class hierarchy.  Perhaps we
> should turn TextConflict into a subclass of ContentsConflict.

Possibly. I do see your point that Text is just a type of Content, so it
can be considered a specialization.

John
=:->