[PATCH] api-credential.txt: document that helpers field is filled-in automatically

[PATCH] api-credential.txt: document that helpers field is filled-in automatically

[Matthieu Moy]

It was unclear whether the field was to be specified by the user of the
API.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

A quick grep in Git's source code seems to show that no user of the
API explicitely modify this "helpers" field (except test-credential.c).

 Documentation/technical/api-credentials.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index 21ca6a2..fc6d2b9 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -46,7 +46,9 @@ Functions
 	consulting helpers, then asking the user. After this function
 	returns, the username and password fields of the credential are
 	guaranteed to be non-NULL. If an error occurs, the function will
-	die().
+	die(). The `helpers` member of the structure is filled-in
+	according to the corresponding configuration variables before
+	consulting helpers.
 
 `credential_reject`::
 
-- 
1.7.11.rc0.57.g84a04c7

Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> It was unclear whether the field was to be specified by the user of the
> API.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>
> A quick grep in Git's source code seems to show that no user of the
> API explicitely modify this "helpers" field (except test-credential.c).

It is correct that the C API asks helpers that the user configured,
but I think it is common across three API functions, not limited to
credential_fill().  credential_apply_config() is called from approve
and reject, too.

>  Documentation/technical/api-credentials.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
> index 21ca6a2..fc6d2b9 100644
> --- a/Documentation/technical/api-credentials.txt
> +++ b/Documentation/technical/api-credentials.txt
> @@ -46,7 +46,9 @@ Functions
>  	consulting helpers, then asking the user. After this function
>  	returns, the username and password fields of the credential are
>  	guaranteed to be non-NULL. If an error occurs, the function will
> -	die().
> +	die(). The `helpers` member of the structure is filled-in
> +	according to the corresponding configuration variables before
> +	consulting helpers.
>  
>  `credential_reject`::

[PATCH] api-credential.txt: document that helpers field is filled-in automatically

[Matthieu Moy]

It was unclear whether the field was to be specified by the user of the
API.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
> It is correct that the C API asks helpers that the user configured,
> but I think it is common across three API functions, not limited to
> credential_fill().  credential_apply_config() is called from approve
> and reject, too.

Ah, right, so compared to v1, we can move the sentence to the
description of the "helpers" field, like this:

 Documentation/technical/api-credentials.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index 21ca6a2..48f51ed 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -21,7 +21,9 @@ Data Structures
 The `helpers` member of the struct is a `string_list` of helpers.  Each
 string specifies an external helper which will be run, in order, to
 either acquire or store credentials. See the section on credential
-helpers below.
+helpers below. This list is filled-in by the API functions
+according to the corresponding configuration variables before
+consulting helpers.
 +
 This struct should always be initialized with `CREDENTIAL_INIT` or
 `credential_init`.
-- 
1.7.11.rc0.57.g84a04c7

Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically

[Jeff King]

On Mon, Jun 11, 2012 at 07:51:47PM +0200, Matthieu Moy wrote:

> It was unclear whether the field was to be specified by the user of the
> API.
> 
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> > It is correct that the C API asks helpers that the user configured,
> > but I think it is common across three API functions, not limited to
> > credential_fill().  credential_apply_config() is called from approve
> > and reject, too.
> 
> Ah, right, so compared to v1, we can move the sentence to the
> description of the "helpers" field, like this:

I think this is OK. Technically it loads other matching config, as well
(filling in default username fields and respecting useHttpPath). I don't
know if it is worth mentioning them, too.

> diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
> index 21ca6a2..48f51ed 100644
> --- a/Documentation/technical/api-credentials.txt
> +++ b/Documentation/technical/api-credentials.txt
> @@ -21,7 +21,9 @@ Data Structures
>  The `helpers` member of the struct is a `string_list` of helpers.  Each
>  string specifies an external helper which will be run, in order, to
>  either acquire or store credentials. See the section on credential
> -helpers below.
> +helpers below. This list is filled-in by the API functions
> +according to the corresponding configuration variables before
> +consulting helpers.

You might want to say something like "...and therefore there is usually
no need for a caller to modify the helpers field at all".

-Peff

Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> On Mon, Jun 11, 2012 at 07:51:47PM +0200, Matthieu Moy wrote:
>
>> It was unclear whether the field was to be specified by the user of the
>> API.
>> 
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>> > It is correct that the C API asks helpers that the user configured,
>> > but I think it is common across three API functions, not limited to
>> > credential_fill().  credential_apply_config() is called from approve
>> > and reject, too.
>> 
>> Ah, right, so compared to v1, we can move the sentence to the
>> description of the "helpers" field, like this:
>
> I think this is OK. Technically it loads other matching config, as well
> (filling in default username fields and respecting useHttpPath). I don't
> know if it is worth mentioning them, too.

Probably not, but I don't care much here. The "helpers" field was
particular in that by reading the documentation, I first thought that
the caller may have a reason to fill-in this field.

>> -helpers below.
>> +helpers below. This list is filled-in by the API functions
>> +according to the corresponding configuration variables before
>> +consulting helpers.
>
> You might want to say something like "...and therefore there is usually
> no need for a caller to modify the helpers field at all".

Agreed. I see that Junio already has this fixup in pu. Junio, let me
know if you prefer a resend.

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

Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Agreed. I see that Junio already has this fixup in pu. Junio, let me
> know if you prefer a resend.

I take it that you told me to squash that fixup in, so will do.
It's much easier for me ;-)

Thanks.