Page MenuHomePhabricator

error: unify the error message formats for 'rebase' and 'unshelve'
ClosedPublic

Authored by dploch on Jul 10 2020, 9:04 PM.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dploch created this revision.Jul 10 2020, 9:04 PM
dploch updated this revision to Diff 21872.Mon, Jul 13, 7:46 PM

Er, well... I guess reordering the chain broke phabricator? Originally I had A -> B -> C, as suggested I reordered this to B -> A -> C with appropriate changes, and how hg phabsend crashes with:

abort: Conduit Error (ERR-CONDUIT-CORE): Graph cycle detected (type=5, cycle=PHID-DREV-jpk3ymcax5ovexpph646, PHID-DREV-ft6irv5eikzqgw4mezxq, PHID-DREV-jpk3ymcax5ovexpph646)

Is this a known error? Should I try abandoning all three commits and re-uploading them?

Er, well... I guess reordering the chain broke phabricator? Originally I had A -> B -> C, as suggested I reordered this to B -> A -> C with appropriate changes, and how hg phabsend crashes with:

abort: Conduit Error (ERR-CONDUIT-CORE): Graph cycle detected (type=5, cycle=PHID-DREV-jpk3ymcax5ovexpph646, PHID-DREV-ft6irv5eikzqgw4mezxq, PHID-DREV-jpk3ymcax5ovexpph646)

Is this a known error? Should I try abandoning all three commits and re-uploading them?

I think that's a know problem (known to me anyway :)). You can fix it here in Phabricator's UI. I'll do that for you.

! In D8730#130363, @martinvonz wrote:
I think that's a know problem (known to me anyway :)). You can fix it here in Phabricator's UI. I'll do that for you.

Thanks, that fixed it :)

+1 for unification. Should we try to align on the message from hg merge` instead?

+1 for unification. Should we try to align on the message from hg merge` instead?

You mean this one?

0 files updated, 0 files merged, 0 files removed, 1 files unresolved
use 'hg resolve' to retry unresolved file merges or 'hg merge --abort' to abandon

The file stats may be useful. I suppose we have that information available here too (but out of scope for this patch). I'm surprised that the text part doesn't mention running hg commit to continue the merge. It seems like it should.

Update also have some clearer precursors message. eg:

warning: conflicts while merging zzz2_merge_bad! (edit, then use 'hg resolve --mark')
2 files updated, 1 files merged, 3 files removed, 1 files unresolved
use 'hg resolve' to retry unresolved file merges

My general point is that if we are to unify and improve merge conflict related message, we should probably align things on what the main command for merging is: hg merge. (Possibly improving it in the process).

Update also have some clearer precursors message. eg:

warning: conflicts while merging zzz2_merge_bad! (edit, then use 'hg resolve --mark')
2 files updated, 1 files merged, 3 files removed, 1 files unresolved
use 'hg resolve' to retry unresolved file merges

My general point is that if we are to unify and improve merge conflict related message, we should probably align things on what the main command for merging is: hg merge. (Possibly improving it in the process).

Right, I agree with that, but I also don't want that to be a requirement for the series to get accepted. In other words, I'd be happy to see that larger unification, but I'd leave it up to Daniel to decide if it's worth his time.

Update also have some clearer precursors message. eg:

warning: conflicts while merging zzz2_merge_bad! (edit, then use 'hg resolve --mark')
2 files updated, 1 files merged, 3 files removed, 1 files unresolved
use 'hg resolve' to retry unresolved file merges

My general point is that if we are to unify and improve merge conflict related message, we should probably align things on what the main command for merging is: hg merge. (Possibly improving it in the process).

Right, I agree with that, but I also don't want that to be a requirement for the series to get accepted. In other words, I'd be happy to see that larger unification, but I'd leave it up to Daniel to decide if it's worth his time.

I'm happy to tweak the error message for rebase+unshelve to be closer to 'merge' if that makes a larger unification easier, but I'm not sure what change is actually being proposed - merge is also fundamentally different because there is no hg merge --continue. It seems it would be better to change the merge message to more closely resemble the other state messages, but that change should probably be in a separate patch.

pulkit accepted this revision.Fri, Jul 17, 1:05 PM
pulkit added a subscriber: pulkit.

Update also have some clearer precursors message. eg:

warning: conflicts while merging zzz2_merge_bad! (edit, then use 'hg resolve --mark')
2 files updated, 1 files merged, 3 files removed, 1 files unresolved
use 'hg resolve' to retry unresolved file merges

My general point is that if we are to unify and improve merge conflict related message, we should probably align things on what the main command for merging is: hg merge. (Possibly improving it in the process).

Right, I agree with that, but I also don't want that to be a requirement for the series to get accepted. In other words, I'd be happy to see that larger unification, but I'd leave it up to Daniel to decide if it's worth his time.

I'm happy to tweak the error message for rebase+unshelve to be closer to 'merge' if that makes a larger unification easier, but I'm not sure what change is actually being proposed - merge is also fundamentally different because there is no hg merge --continue. It seems it would be better to change the merge message to more closely resemble the other state messages, but that change should probably be in a separate patch.

Agree with Daniel that there is no hg merge --continue and hence won't be trivial to merge messages. Anyways, this is an improvement which is good to go on it's own, hence queuing it. Thank you!

This revision is now accepted and ready to land.Fri, Jul 17, 1:05 PM