Thread (2 messages) flat view 2 messages, 2 authors, 2021-11-19

Re: [PATCH v6 3/3] am: throw an error when passing --empty option without value

From: Aleen 徐沛文 <hidden>
Date: 2021-11-19 02:11:12

Here is a place for you to explain why this change might be a good
idea to defend it.  If we applied 1/3 and 2/3 but not this one, what
happens?  "am --empty" (not "am --empty=<choice>") is accepted and
behaves just like "am --empty=die"?  Is that a bad thing?

If it is unconditonally a bad thing, then this patch does not stand
on its own.  It should be squashed into the previous step; otherwise
it would be "oops, we didn't think it through and failed to detect
an argument-less --error as an error in the previous step, and here
is a fix for that mistake".  We try not to deliberately commit a
mistake and then fix it to gain commit counts in this project (or
putting it differently, we try to pretend we are better than we
really are and did not make a mistake in the first place).

Or are you giving list participants and reviewers a choice between
"'--empty' is a synonym to '--empty=die'" and "'--empty' alone is an
error" because you cannot decide yourself?  If that is the case, you
really really need to make it clear in this place between "From:"
and "Signed-off-by:".  Why would we want to use this patch?  Why
would we be better without this patch?

I haven't thought things through, but my gut feeling is that the
semantics of the --empty=<choice> option is better _with_ this,
in other words, it makes more sense to forbid "--empty" alone.  So
if I were doing this series, I would probably have squashed this
into the previous one, making it a two-patch series.

Thanks.
Hamano, I could not make this decision like what you said between "'--empty' is a synonym to '--empty=die'" and "'--empty' alone is an error". In my opinion, it is a little wired that '--empty' should not act as "died" semantically. Reversely, it may be better to support a synonym between '--quiet' and '--empty=skip'.

I will squash the commit into the previous one later and hope all participants and reviewers make a decision.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help