Re: [PATCH 6/9] builtin-init-db.c: use parse_options()

Hi,

On Thu, 24 Jul 2008, Olivier Marin wrote:

Michele Ballabio a ??rit :

quoted

+		OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"),

s/set up/setup/

No.  "setup" is a noun.

quoted

+		{ OPTION_CALLBACK, 0, "shared", &shared_repository,
+		  "type", "type of shared repository",

What about "permissions", "setup a shared repository"?

quoted

+		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },

Are you sure the default value is really used here?

Also, perhaps we can play it safer by avoiding changing "share_repository"
directly.

I do not see how that would be any safer.

$ git init -> shared_repository == PERM_UMASK
$ git init --shared --no-shared -> shared_repository == 0

It works because PERM_UMASK == 0, but it is a side effect. Don't you think?

Then the callback is wrong, too.  I think, however, that it is by design, 
and correct.

We rely on shared_repository == 0 for non-shared repositories _almost 
everywhere_.

quoted

+		OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),

OPT__QUIET(&quiet),

if (quiet)
	flags |= INIT_DB_QUIET;

to use the same quiet option everywhere?

Why?  Doesn't make it more readable, I think.  I'd rather have 3 lines 
less.

Ciao,
Dscho