On Fri, May 27, 2016 at 10:16 AM, Antoine Queru
<[email protected]> wrote:
> upload-pack.c: use of parse-options API
Matthieu already mentioned that this should use imperative mood:
upload-pack: use parse-options API
> Option parsing now uses the parser API instead of a local parser.
> Code is now more compact.
Imperative:
Use the parse-options API rather than a hand-rolled
option parser.
That the code becomes more compact is a nice result of this change,
but not particularly important, thus not really worth a mention in the
commit message.
> Description for --stateless-rpc and --advertise-refs come from
> the commit 42526b4 (Add stateless RPC options to upload-pack,
> receive-pack, 2009-10-30).
s/from the commit/from/
> Signed-off-by: Antoine Queru <[email protected]>
> Signed-off-by: Matthieu Moy <[email protected]>
> ---
> diff --git a/Documentation/git-upload-pack.txt
> b/Documentation/git-upload-pack.txt
> @@ -31,6 +31,19 @@ OPTIONS
> +--stateless-rpc::
> + the programs now assume they may perform only a single read-write
s/the/The/
Also, to what "programs" does this refer? And what does "now" mean here?
> + cycle with stdin and stdout. This fits with the HTTP POST request
> + processing model where a program may read the request, write a
> + response, and must exit.
> +
> +--advertise-refs::
> + When --advertise-refs is passed as a command line parameter only
The entire "When ... parameter" bit is redundant, isn't it? Why not
just say "Perform only..."?
> + the initial ref advertisement is output, and the program exits
> + immediately. This fits with the HTTP GET request model, where
> + no request content is received but a response must be produced.
> +
> +
Style: Drop the extra blank line.
> <directory>::
> The repository to sync from.
>
> diff --git a/upload-pack.c b/upload-pack.c
> @@ -817,11 +821,21 @@ static int upload_pack_config(const char *var, const
> char *value, void *unused)
> -int main(int argc, char **argv)
> +int main(int argc, const char **argv)
> {
> - char *dir;
> - int i;
> + const char *dir;
> int strict = 0;
> + struct option options[] = {
> + OPT_BOOL(0, "stateless-rpc", &stateless_rpc,
> + N_("quit after a single request/response exchange")),
> + OPT_BOOL(0, "advertise-refs", &advertise_refs,
> + N_("only the initial ref advertisement is output,
> program exits immediately")),
Possible rewrite: "exit immediately after initial ref advertisement"
> + OPT_BOOL(0, "strict", &strict,
> + N_("do not try <directory>/.git/ if <directory> is
> no Git directory")),
Use of OPT_BOOL introduces a --no-strict option which didn't exist
before. Does this need to be documented? (Genuine question.)
> + OPT_INTEGER(0, "timeout", &timeout,
> + N_("interrupt transfer after <n> seconds of
> inactivity")),
> + OPT_END()
> + };
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
