Re: [PATCH 1/4] sequencer: Do not require `allow_empty` for redundant commit options

["Brian Lyles" <brianmlyles@gmail.com>]

Hi Phillip

On Thu, Feb 1, 2024 at 4:57 AM Phillip Wood <phillip.wood123@gmail.com>
wrote:

> That sounds like a good strategy. I'm wondering if we should not change 
> the behavior of `--keep-redundant-commits` to avoid breaking existing 
> users but have `--empty=keep|drop` not imply `--allow-empty`. What ever 
> we do we'll annoy someone. It is confusing to have subtly different 
> behaviors for `--keep-redundant-commits` and `--empty=keep` but it 
> avoids breaking existing users. If we change `--keep-redundant-commits` 
> we potentially upset existing users but we don't confuse others with the 
> subtle difference between the two.

I am starting to come around to this approach for this particular series
just to avoid anything potentially controversial holding it up.

>>> Do you have a practical example of where you want to keep the commits
>>> that become empty but not the ones that start empty? I agree there is a
>>> distinction but I think the common case is that the user wants to keep
>>> both types of empty commit or none. I'm not against giving the user the
>>> option to keep one or the other if it is useful but I'm wary of changing
>>> the default.
>> 
>> That practical example is documented in the initial discussion[1], which
>> I should have ought to have linked in a cover letter for this series
>> (and will do so in v2). I'll avoid copying the details here, but we'd
>> very much like to be able to programmatically drop the commits that
>> become empty when doing the automated cherry-pick described there.
>> 
>> [1]: https://lore.kernel.org/git/CAHPHrSevBdQF0BisR8VK=jM=wj1dTUYEVrv31gLerAzL9=Cd8Q@mail.gmail.com/
> 
> Maybe I've missed something but that seems to be an argument for 
> implementing `--empty=drop` which is completely reasonable but doesn't 
> explain why someone using `--keep-redundant-commits` would want to keep 
> the commits that become empty while dropping the commits that start empty.

Nope, you didn't miss something -- I just didn't read properly.

I don't have a concrete example here, but the behavior *feels* quite odd
to me. But I suppose that's not a good enough reason to make a breaking
change.

-- 
Thank you,
Brian Lyles