[v2,3/3] status: mention --skip for revert and cherry-pick
diff mbox series

Message ID be64ce1e92c60f9587b137d36e98532604d4a1ff.1566880835.git.liu.denton@gmail.com
State New
Headers show
Series
  • advertise --skip for cherry-pick and revert better
Related show

Commit Message

Denton Liu Aug. 27, 2019, 4:45 a.m. UTC
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(+)

Comments

Junio C Hamano Aug. 27, 2019, 9:56 p.m. UTC | #1
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?
Denton Liu Aug. 27, 2019, 11:18 p.m. UTC | #2
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 Aug. 28, 2019, 2:47 a.m. UTC | #3
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".
Denton Liu Aug. 28, 2019, 6:45 a.m. UTC | #4
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.

Patch
diff mbox series

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)"));
 	}