Re: [PATCH 3/6] revert: don't let revert continue a cherry-pick
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:52:44
Ramkumar Ramachandra wrote:
Jonathan Nieder wrote:
quoted
I don't know --- it's not confusing to me. Could you explain further what harm the current behavior does? E.g., could it cause me to misunderstand some basic concepts, or could it lead me to run commands that cause me to scratch my head or lose data?Junio explained this to me in [1]. It's very unnatural for a user to want to execute "git cherry-pick --continue" when the previous command was a "git revert": it probably means that she forgot about the in-progress "git revert".
[...]
[1]: http://thread.gmane.org/gmane.comp.version-control.git/185355
I don't think that's what Junio said. Did this actually happen, or is it a theoretical worry? I think I would be more likely to run "git cherry-pick <foo>..<bar>" than "git cherry-pick --continue" if I had forgotten about an in-progress revert. The former already errors out with a sensible message. Or is the problem that I might run: git revert foo..bar git reset --merge; # conflict --- let's clean this up # ah, I remember reverting the patch that conflicted before; # let's reuse the resolution. git cherry-pick baz edit file.c; # another conflict, sigh git add file.c git cherry-pick --continue; # oops! ? That seems like a real worry, but the same problem could happen with cherry-pick used both for the multipick and single-pick, so I don't think your patch fundamentally addresses it. In other words, this is a problem caused by the overloading of the same cherry-pick command for single-pick and multi-pick. I think it should be preventable by remembering which action failed when stopping a sequence and doing only a single-pick resume if CHERRY_PICK_HEAD/REVERT_HEAD/whatever doesn't match that. The "oops" is bad since the operator might have been intending to run some more tests and amend as necessary before continuing the multi-pick. It is not _that_ bad, since more typically one would have already run some tests before running cherry-pick --continue to commit the resolution. Still probably worth fixing.
The problem becomes more serious when the sequencer grows more capabilities: a "git merge --continue" to continue a "git am" sounds much more absurd. Ofcourse, we will provide a way to continue any sequencer operation in the future: "git continue" seems to be a good candidate.
I don't understand why "cherry-pick --continue" resuming a revert sequence implies that "merge --continue" would have to as well. All that said, forbidding cherry-pick --continue from resuming a revert sequence would be fine with me, _as long as the semantics are clearly spelled out in the commit message and documentation_. What happens when there is a mixture of picks and reverts? Thanks. Jonathan