Re: [RFC PATCH v2 04/12] submodule--helper: add a new 'config' subcommand

> On Thu, Aug 02, 2018 at 11:47:30AM -0700, Stefan Beller wrote:
> 
> > > +static int module_config(int argc, const char **argv, const char *prefix)
> > > +{
> > > +       if (argc < 2 || argc > 3)
> > > +               die("submodule--helper config takes 1 or 2 arguments: name [value]");
> > > +
> > > +       /* Equivalent to ACTION_GET in builtin/config.c */
> > > +       if (argc == 2)
> > > +               return print_config_from_gitmodules(argv[1]);
> > > +
> > > +       /* Equivalent to ACTION_SET in builtin/config.c */
> > > +       if (argc == 3)
> > > +               return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
> > > +
> > > +       return 0;
> > 
> > Technically we cannot reach this point here?
> > Maybe it would be more defensive to
> > 
> >     BUG("How did we get here?");
> > 
> > or at least return something !=0 ?
> 
> When I find myself reaching for a BUG(), it is often a good time to see
> if we can restructure the code so that the logic more naturally falls
> out.  Here the issue is that the first if conditional repeats the "else"
> for the other two. So I think we could just write:
> 
>   if (argc == 2)
> 	return ...
>   if (argc == 3)
> 	return ...
> 
>   die("need 1 or 2 arguments");
>