From: phillip.wood123@gmail.com Date: Wed, 20 Mar 2024 19:35:12 GMT Subject: Re: [PATCH v3 7/7] cherry-pick: add `--empty` for more robust redundant commit handling Message-ID: <507f8792-5f79-4ab3-b328-abfbc5dbffc5@gmail.com> In-Reply-To: Hi Brian On 16/03/2024 05:20, Brian Lyles wrote: > On Wed, Mar 13, 2024 at 12:17 PM Junio C Hamano wrote: >> phillip.wood123@gmail.com writes: >>>> +Note that this option specifies 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`. >>> >>> I found this last paragraph is slightly confusing now --empty=keep >>> implies --allow-empty. Maybe we could change the middle sentence to >>> say something like >>> >>> With the exception of `--empty=keep` commits that were initially >>> empty will cause the cherry-pick to fail. >> >> That is certainly easier to read and much less confusing. > > I agree that this paragraph is slightly confusing. I tried this > suggestion on but found it to not sit quite right, I think because the > two exceptions (--empty=keep and --allow-empty) were not part of the > same sentence, so it felt a little disjointed. How would you feel about > the following instead, which aims to be more clear and specific about > the behavior? > > Note that `--empty=drop` and `--empty=stop` only specify 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 still cause the cherry-pick to fail unless one of > `--empty=keep` or `--allow-empty` are specified. That looks fine to me Best Wishes Phillip