On Thursday 24 July 2008, Olivier Marin wrote:
> 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.
Ok.
> > + OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"),
>
> s/set up/setup/ and s/repo/repository/?
I think "set up a bare repository" will be fine.
> > + { OPTION_CALLBACK, 0, "shared", &shared_repository,
> > + "type", "type of shared repository",
>
> What about "permissions", "setup a shared repository"?
Ok, but with s/setup/set up/.
> > + PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
>
> Are you sure the default value is really used here?
Yes. Perhaps I don't understand your question. Can you explain what you mean?
> 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?
Would you like this better, with PARSE_OPT_NONEG?
+ { OPTION_CALLBACK, 0, "shared", &shared_repository,
+ "permissions", "set up a shared repository",
+ PARSE_OPT_OPTARG | PARSE_OPT_NONEG, parse_opt_shared_cb, PERM_GROUP },
Or do you prefer changing the callback like this:
+static int parse_opt_shared_cb(const struct option *opt, const char *arg,
+ int unset)
+{
+ *(int *)(opt->value) = unset ? PERM_UMASK : git_config_perm("arg", arg);
+ return 0;
+}
> > + 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?
I thought about it and decided against ;)
And it's one line vs four (counting "int quiet = 0;"). But I see your point.
--
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
