Jeff King wrote:
> On Tue, Nov 14, 2023 at 11:38:19AM -0500, Todd Zullinger wrote:
>> I've run this through the full test suite. I also compared the output of
>> --help to ensure it only differs in the removal of the "Duplicate
>> specification" warnings. I _think_ that's a good sign that no other changes
>> will result. But I would be grateful to anyone who can confirm or reject that
>> theory.
>
> I guess you meant "-h", not "--help", since the latter will just show
> the manpage. But isn't "-h" just dumping a static usage message we
> wrote, and not auto-generated by the code?
Yes to both. This is why I shouldn't submit patches within
a few hours of waking up.
> The changes look good to me (even after double-checking Junio's question
> that they are all appropriately matched with their "positive" sides).
Indeed. I need to go through them each to test that the
results match before and after. With the fallback to
passing options to format-patch, testing outside of a git
repo makes this rather convenient. If I've dropped an
option it will result in the "Cannot run git format-patch
from outside a repository" error. That's a good start to
ensure the changes don't cause any regressions.
I did notice that I mistakenly dropped --[no-]signed-off-cc.
I need to keep:
"signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
as is.
> This one is curious:
>
>> - "cc-cover|cc-cover!" => \$cover_cc,
>
> It was an alternate name for itself? I think somebody just misunderstood
> how the API was supposed to work. The "!" would applies to all names, if
> I understand correctly, so this really is doing nothing beyond just
> "cc-cover!", which is what your patch switches it to.
I wondered about those as well. Perhaps this is needed in
some older version of Getopt::Long? I'll try to look
through the history of the module to see if that's the case.
Since this isn't anything new with 2.43, it doesn't need to
be fixed with much urgency.
Thanks both,
--
Todd
