From: Olivier Marin <dkr+ml.git@free.fr>
Date: Thu, 24 Jul 2008 16:15:46 GMT
Subject: Re: [PATCH 6/9] builtin-init-db.c: use parse_options()
Message-ID: <4888AAB2.5050007@free.fr>
In-Reply-To: <1216849332-26813-7-git-send-email-barra_cuda@katamail.com>

Michele Ballabio a écrit :
> 
> +	const struct option options[] = {
> +		OPT_STRING(0, "template", &template_dir, "dir",
> +			   "directory from which templates will be used"),

Perhaps "path", "path to the template repository" to stay consistent with clone.

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

s/set up/setup/ and s/repo/repository/?

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

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

> +		  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.

$ 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?

> +		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?

Just my opinion,
Olivier.