Re: [PATCH] Use OPT_CALLBACK and OPT_CALLBACK_F

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 28, 2020, 17:34 UTC
Message-ID: <xmqq368n4iql.fsf@gitster.c.googlers.com>
In-Reply-To: <20200428084543.GC2381876@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> On Tue, Apr 28, 2020 at 04:36:28AM -0400, Denton Liu wrote:
>
>> In the codebase, there are many options which use OPTION_CALLBACK in a
>> plain ol' struct definition. However, we have the OPT_CALLBACK and
>> OPT_CALLBACK_F macros which are meant to abstract these plain struct
>> definitions away. These macros are useful as they semantically signal to
>> developers that these are just normal callback option with nothing fancy
>> happening.
>
> I think this is worth doing. It's a little easier to read, and sets a
> better example anyone copying the code.

Sure.

And it would be much nicer if the patch actually applies. I am getting "SHA-1 information for builtin/push.c is unusable" from "am -3".

I'd reject the first hunk on builtin/push.c as that part should be done with an updated version of the patch that is titled "push: unset PARSE_OPT_OPTARG for --recurse-submodules" and apply the rest as an independent clean-up patch.

Thanks, both.