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