| Message ID | be64ce1e92c60f9587b137d36e98532604d4a1ff.1566880835.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | advertise --skip for cherry-pick and revert better | expand |
Denton Liu <liu.denton@gmail.com> writes: > When reverting or cherry-picking, one of the options we can pass the > sequencer is `--skip`. However, unlike rebasing, `--skip` is not > mentioned as a possible option in the status message. Mention it so that > users are more aware of their options. Is this a good thing, though? Giving up (because you do not have enough time or concentration to finish the cherry-pick or revert in progress) with --abort, and committing to the resolution after spending effort to deal with a conflicted cherry-pick or revert with --continue, are both sensible actions after seeing the command stop due to conflicts. Is "--skip" a recommendable action in the same way? Doesn't a multi-commit series often break if you drop just one in the middle, especially if the series is sensibly structured as a logical progression?
On Tue, Aug 27, 2019 at 02:56:57PM -0700, Junio C Hamano wrote: > Denton Liu <liu.denton@gmail.com> writes: > > > When reverting or cherry-picking, one of the options we can pass the > > sequencer is `--skip`. However, unlike rebasing, `--skip` is not > > mentioned as a possible option in the status message. Mention it so that > > users are more aware of their options. > > Is this a good thing, though? > > Giving up (because you do not have enough time or concentration to > finish the cherry-pick or revert in progress) with --abort, and > committing to the resolution after spending effort to deal with a > conflicted cherry-pick or revert with --continue, are both sensible > actions after seeing the command stop due to conflicts. Is "--skip" > a recommendable action in the same way? Doesn't a multi-commit > series often break if you drop just one in the middle, especially > if the series is sensibly structured as a logical progression? I think that the same argument for or against recommending `--skip` could be made for rebases as well. However, in the rebase case, `--skip` is recommended whenever `--abort` is recommended. With this patch, I made it so that revert and cherry-pick would have `--skip` and `--abort` paired as well. I'm pretty impartial about making this change but I would suggest if we choose not to pursue this then we should also drop the `--skip` recommendation from rebase as well.
Junio C Hamano <gitster@pobox.com> writes: > Is this a good thing, though? > > Giving up (because you do not have enough time or concentration to > finish the cherry-pick or revert in progress) with --abort, and > committing to the resolution after spending effort to deal with a > conflicted cherry-pick or revert with --continue, are both sensible > actions after seeing the command stop due to conflicts. Is "--skip" > a recommendable action in the same way? Doesn't a multi-commit > series often break if you drop just one in the middle, especially > if the series is sensibly structured as a logical progression? Addendum. "rebase" (especially with "-i") is fundamentally different from "cherry-pick" and it makes tons of sense to suggest "--skip" in the former. "rebase -i" is a tool to take a messy work in progress and polish it by reordering, discarding and combining commits. "cherry-pick" is to take a finished work already in one integration track, and transplant to another, often an older maintenance track, and there is no place for "this conflict is too much to resolve so let's drop it".
On Tue, Aug 27, 2019 at 07:47:33PM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > Is this a good thing, though? > > > > Giving up (because you do not have enough time or concentration to > > finish the cherry-pick or revert in progress) with --abort, and > > committing to the resolution after spending effort to deal with a > > conflicted cherry-pick or revert with --continue, are both sensible > > actions after seeing the command stop due to conflicts. Is "--skip" > > a recommendable action in the same way? Doesn't a multi-commit > > series often break if you drop just one in the middle, especially > > if the series is sensibly structured as a logical progression? > > Addendum. > > "rebase" (especially with "-i") is fundamentally different from > "cherry-pick" and it makes tons of sense to suggest "--skip" in the > former. "rebase -i" is a tool to take a messy work in progress and > polish it by reordering, discarding and combining commits. > "cherry-pick" is to take a finished work already in one integration > track, and transplant to another, often an older maintenance track, > and there is no place for "this conflict is too much to resolve so > let's drop it". > I still believe that it's a good idea to let users know what all of their options are, including the --skip for cherry-pick and revert even if it may be rare that it's the right thing to do. That being said, I'm not passionate enough about this change to pursue it so when you queue this patchset, please drop this patch.
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index e01c285cbf..66d7a62797 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -733,6 +733,7 @@ test_expect_success 'status when cherry-picking before resolving conflicts' ' On branch cherry_branch You are currently cherry-picking commit $TO_CHERRY_PICK. (fix conflicts and run "git cherry-pick --continue") + (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) Unmerged paths: @@ -757,6 +758,7 @@ test_expect_success 'status when cherry-picking after resolving conflicts' ' On branch cherry_branch You are currently cherry-picking commit $TO_CHERRY_PICK. (all conflicts fixed: run "git cherry-pick --continue") + (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) Changes to be committed: @@ -778,6 +780,7 @@ test_expect_success 'status when cherry-picking after committing conflict resolu On branch cherry_branch Cherry-pick currently in progress. (run "git cherry-pick --continue" to continue) + (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) nothing to commit (use -u to show untracked files) @@ -835,6 +838,7 @@ test_expect_success 'status while reverting commit (conflicts)' ' On branch master You are currently reverting commit $TO_REVERT. (fix conflicts and run "git revert --continue") + (use "git revert --skip" to skip this patch) (use "git revert --abort" to cancel the revert operation) Unmerged paths: @@ -855,6 +859,7 @@ test_expect_success 'status while reverting commit (conflicts resolved)' ' On branch master You are currently reverting commit $TO_REVERT. (all conflicts fixed: run "git revert --continue") + (use "git revert --skip" to skip this patch) (use "git revert --abort" to cancel the revert operation) Changes to be committed: @@ -887,6 +892,7 @@ test_expect_success 'status while reverting after committing conflict resolution On branch master Revert currently in progress. (run "git revert --continue" to continue) + (use "git revert --skip" to skip this patch) (use "git revert --abort" to cancel the revert operation) nothing to commit (use -u to show untracked files) diff --git a/wt-status.c b/wt-status.c index 9f6c65a580..ad6282c7f8 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1433,6 +1433,8 @@ static void show_cherry_pick_in_progress(struct wt_status *s, else status_printf_ln(s, color, _(" (all conflicts fixed: run \"git cherry-pick --continue\")")); + status_printf_ln(s, color, + _(" (use \"git cherry-pick --skip\" to skip this patch)")); status_printf_ln(s, color, _(" (use \"git cherry-pick --abort\" to cancel the cherry-pick operation)")); } @@ -1460,6 +1462,8 @@ static void show_revert_in_progress(struct wt_status *s, else status_printf_ln(s, color, _(" (all conflicts fixed: run \"git revert --continue\")")); + status_printf_ln(s, color, + _(" (use \"git revert --skip\" to skip this patch)")); status_printf_ln(s, color, _(" (use \"git revert --abort\" to cancel the revert operation)")); }
When reverting or cherry-picking, one of the options we can pass the sequencer is `--skip`. However, unlike rebasing, `--skip` is not mentioned as a possible option in the status message. Mention it so that users are more aware of their options. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t7512-status-help.sh | 6 ++++++ wt-status.c | 4 ++++ 2 files changed, 10 insertions(+)