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

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 11, 2012, 19:04 UTC
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".

Show 9 quoted lines

> --- /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)

Show 7 quoted lines

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