Thread (2 messages) 2 messages, 2 authors, 2024-01-28

Re: [PATCH 4/4] cherry-pick: Add `--empty` for more robust redundant commit handling

From: Brian Lyles <hidden>
Date: 2024-01-28 00:08:14

Hi Junio

On Tue, Jan 23, 2024 at 12:01 PM Junio C Hamano [off-list ref] wrote:
Phillip Wood [off-list ref] writes:
quoted
Thanks for the well explained commit message
;-)
quoted
quoted
The `--keep-redundant-commits` option will be documented as a deprecated
synonym of `--empty=keep`, and will be supported for backwards
compatibility for the time being.
I'm not sure if we need to deprecate it as in "it will be removed in
the future" or just reduce it prominence in favor of --empty
True, especially since --empty=keep is much less descriptive and the
part after "note that ..." below will take a long time before
sticking in readers' brain.
I responded to this in my reply to Phillip, and CC'd you there.
quoted
quoted
+--empty=(stop|drop|keep)::
+    How to handle commits being cherry-picked that are redundant with
+    changes already in the current history.
It might make it easier to understand if we moved the description in
'keep' that says something about --allow-empty here, as it should
apply to other two choices if I understand correctly.  Let me try:

    This option specifies how a commit that is not originally empty
    but becomes a no-op when cherry-picked due to earlier changes
    already applied or already in the current history.  Regardless
    of this this option, `cherry-pick` will fail on a commit that is
    empty in the original history---see `--allow-empty` to pass them
    intact.

or something.  Then the description of `keep` can become just as
short as other two, e.g. a single sentence "The commit that becomes
empty will be kept".
Thank you for this suggestion. You are correct that the difference
between `--empty` and `allow-empty` is relevant regardless of the option
selected by the user.

In fact, this entire tidbit is somewhat duplicative with the note I
already added after the options:
Note that commits which start empty will cause the cherry-pick to fail
(unless `--allow-empty` is specified).
I'll clean this up in v2. Here's what I am thinking currently:
--empty=(stop|drop|keep)::
    How to handle commits being cherry-picked that are redundant with
    changes already in the current history.
+
--
`stop`;;
    The cherry-pick will stop when the commit is applied, allowing
    you to examine the commit. This is the default behavior.
`drop`;;
    The commit will be dropped.
`keep`;;
    The commit will be kept.
--
+
Note that this option species how to handle a commit that was not initially
empty, but rather became empty due to a previous commit. Commits that were
initially empty will cause the cherry-pick to fail. To force the inclusion of
those commits, use `--allow-empty`.
+
Thank you,
Brian Lyles
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help