From: roucherj <roucherj@telesun.imag.fr>
Date: Mon, 11 Jun 2012 19:12:17 GMT
Subject: Re: [PATCH v3] add 'git credential' plumbing command
Message-ID: <edf83e30636b67d2b95bf1df8519cdcd@telesun.imag.fr>
In-Reply-To: <vpqehpl4qm0.fsf@bauges.imag.fr>

On Mon, 11 Jun 2012 21:04:39 +0200, Matthieu Moy wrote:
>> Subject: Re: [PATCH v3] add 'git credential' plumbing command
>
> I guess you mean v2.
>

Sorry is a miss typing.

> 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.
>

Changed it


> 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, ...).

okay for the next patch i will add (protocol, path, ..)