Re: [PATCH v5 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Johannes,

On Mon, May 9, 2016 at 8:29 PM, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
> Hi Pranit,
>
> On Sun, 8 May 2016, Pranit Bauva wrote:
>
>> On Sun, May 8, 2016 at 12:34 PM, Johannes Schindelin
>> <Johannes.Schindelin@xxxxxx> wrote:
>> >
>> > On Fri, 6 May 2016, Pranit Bauva wrote:
>> >
>> >> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> >> index 3324229..d8de651 100644
>> >> --- a/builtin/bisect--helper.c
>> >> +++ b/builtin/bisect--helper.c
>> >> @@ -8,13 +8,17 @@ static const char * const git_bisect_helper_usage[] = {
>> >>       NULL
>> >>  };
>> >>
>> >> +enum subcommand {
>> >> +     NEXT_ALL = 1
>> >> +};
>> >
>> > I still do not think that this enum needs to have file scope. Function
>> > scope is enough.
>>
>> In the very initial patch I made it in function scope. To which you
>> pointed out[1] that in all other examples but for one have file scope
>> so then I thought maybe that exception was a wrong example and I
>> should stick to the convention of putting it in file scope.
>
> Oh, sorry, I meant to imply that it is good as it is by saying "so this
> code is fine"...
>
> I was just surprised because I thought I remembered that some old C
> standard does not allow enums to be function scoped. But I was wrong.
>
>> But now I also realize that builtin/replace.c uses "cmdmode" instead of
>> "subcommand" so I am still wondering what would be the most appropriate?
>
> I think the replace.c code is really a good example. Function-scoped,
> using the "cmdmode" name that obviously corresponds to the OPT_CMDMODE
> name.

Sure. WIll do!

Regards,
Pranit Bauva
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]