Merge lp://staging/~gz/bzr/resolve_auto_refactor into lp://staging/bzr
| Status: | Needs review |
|---|---|
| Proposed branch: | lp://staging/~gz/bzr/resolve_auto_refactor |
| Merge into: | lp://staging/bzr |
| Diff against target: |
243 lines (+58/-51) 3 files modified
bzrlib/conflicts.py (+36/-28) bzrlib/tests/test_workingtree.py (+13/-6) bzrlib/workingtree.py (+9/-17) |
| To merge this branch: | bzr merge lp://staging/~gz/bzr/resolve_auto_refactor |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| bzr-core | Pending | ||
|
Review via email:
|
|||
Description of the change
Creates a new kind of conflict resolution action named 'auto', refactoring code from `WorkingTree.
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.
Is there documentation other than in conflict-types.txt that this branch should update?
A followup branch will make `bzr resolve FILE` default to --auto rather than --done.
May also be a good idea to unify the --all switch and deprecate it.
I'm not wild about using NotImplementedError as the only mechanism of signalling a conflict as not having been resolved. Perhaps another exception type should be added for this?
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.
Having the pending conflicts returned by the auto_resolve method is actually a little useful. In general, conficts.resolve just returning the counts is fine, but to report the details having the conflict objects is handy.
| Vincent Ladeuil (vila) wrote | # |
Unmerged revisions
- 6547. By Martin Packman
-
Note issue over looking for conflict markers in a removed file
- 6546. By Martin Packman
-
Use tree.get_file rather than touching filesystem directly to auto resolve
- 6545. By Martin Packman
-
Deprecate WorkingTree.
auto_resolve - 6544. By Martin Packman
-
Refactor auto_resolve tree method into auto action on conflicts
> 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-
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).