Code review comment for lp:~gz/bzr/resolve_auto_refactor

> I'd like to write some specific tests for this action type, but can't really see a way in to bt.test_conflicts to do that.

Indeed, IIRC, the tests were added for --take-[this|other]. For them there is no user action.

You need a slight variation where some action is performed by the user after 'merge' but before 'resolve'. In the general case, you want to test for robustness there as really the user may do weird stuff (like deleting some file you expect).

> Is there documentation other than in conflict-types.txt that this branch should update?

I can't remember one.

> Perhaps another exception type should be added for this?

No objection your honor :)

> May also be a good idea to unify the --all switch and deprecate it.

I don't like '--all' (far too many people have used it with disastrous results), so I'll approve its deprecation :)

Now, you may want to get feedback from the list about whether anybody finds it useful *and* used it in the last years. I'm pretty sure it was introduced as a way to get out of weird situations without having to use 'bzr revert'.

> The UI can benefit from some cleanup later, notably `bzr resolve` gives a return code if there are pending conflicts, and there are slightly different spellings of the results.

Yup, look at the annotations, search the mps, the GUI came as an afterthought and was never really designed properly.

> to report the details having the conflict objects is handy.

Leave that to the conflicts objects if you really need to, no need to move them around (especially because the ConflictList object isn't really necessary and makes manipulating the Conflict objects harder than needed).