Re: [PATCH v2] upload-pack.c: use of parse-options API

[Matthieu Moy <matthieu.moy@grenoble-inp.fr>]

Antoine.Queru@ensimag.grenoble-inp.fr wrote:
> Option parsing now uses the parser API instead of a local parser.
> Code is now more compact.
> Description for -stateless-rpc and --advertise-refs
> come from the commit (gmane/131517)

Please, use a real commit id instead of a Gmane link.

We don't know how long Gmane will remain up, but a self
reference from Git's history to itself will always remain valid.

The following alias is handy for this:

[alias]
        whatis = show -s --pretty='tformat:%h (%s, %ad)' --date=short

In your case it would allow writing: 

Description for --stateless-rpc and --advertise-refs is taken
from commit 42526b4 (Add stateless RPC options to upload-pack,
receive-pack, 2009-10-30).

> diff v1 v2:

We usually say "diff" to refer to an actual diff. I'd write "changes since v1" here.

> +		OPT_BOOL(0, "stateless-rpc", &stateless_rpc,
> +			 N_("may perform only a single read-write cycle with stdin and stdout")),

It's weird to define what an option does with "may". It's a
property of --stateless-rpc, but does not really define it.

> +	if (argc != 1)
> +		usage_with_options(upload_pack_usage, options);
>  
> -	setup_path();
> +	if (timeout)
> +		daemon_mode = 1;
>  
> -	dir = argv[i];
> +	setup_path();
>  
> +	dir = argv[0];

Not a problem with your code, but the patch shows "setup_path()"
as moved while it is not really. Maybe using "send-email
--patience" or some other diff option could make the patch nicer.
Not really important as it does not change the final state.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/