From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Mon, 11 Jun 2012 19:04:39 GMT
Subject: Re: [PATCH v3] add 'git credential' plumbing command
Message-ID: <vpqehpl4qm0.fsf@bauges.imag.fr>
In-Reply-To: <1339440294-8010-1-git-send-email-Javier.Roucher-Iglesias@ensimag.imag.fr>

> Subject: Re: [PATCH v3] add 'git credential' plumbing command

I guess you mean v2.

Javier.Roucher-Iglesias@ensimag.imag.fr writes:

> Changes in the version2 vs version1:

These should not go to the commit message, but below the --- below (and
before the diffstat).

> Adding to the next patch, version3:
> · Tests files

If the todo-list isn't empty, then mark your patch as "RFC".

> --- /dev/null
> +++ b/Documentation/git-credential.txt
> @@ -0,0 +1,74 @@
> +git-credential(7)
> +=================
> +
> +NAME
> +----
> +git-credential - Provides and store user credentials to git

Provides -> Provide

I'd remove the "to git" part.

Other than that, I prefer Jeff's version sent yesterday. Any reason not
to use it?

The command still isn't listed in "man git", aka Documentation/git.txt
(I already mentionned it)

> +	if (!strcmp(op, "fill")) {
> +		credential_fill(&c);
> +		if (c.username)
> +			printf("username=%s\n", c.username);
> +		if (c.password)
> +			printf("password=%s\n", c.password);
> +	}

See Jeff's remarks. It makes sense to output all fields here
(protocol, path, ...).

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