* [PATCH] api-credential.txt: document that helpers field is filled-in automatically
@ 2012-06-11 10:46 Matthieu Moy
2012-06-11 16:12 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2012-06-11 10:46 UTC (permalink / raw)
To: git, gitster; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically
2012-06-11 10:46 [PATCH] api-credential.txt: document that helpers field is filled-in automatically Matthieu Moy
@ 2012-06-11 16:12 ` Junio C Hamano
2012-06-11 17:51 ` Matthieu Moy
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2012-06-11 16:12 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
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`::
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] api-credential.txt: document that helpers field is filled-in automatically
2012-06-11 16:12 ` Junio C Hamano
@ 2012-06-11 17:51 ` Matthieu Moy
2012-06-11 19:08 ` Jeff King
0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2012-06-11 17:51 UTC (permalink / raw)
To: git, gitster; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically
2012-06-11 17:51 ` Matthieu Moy
@ 2012-06-11 19:08 ` Jeff King
2012-06-12 9:39 ` Matthieu Moy
0 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2012-06-11 19:08 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically
2012-06-11 19:08 ` Jeff King
@ 2012-06-12 9:39 ` Matthieu Moy
2012-06-12 14:48 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2012-06-12 9:39 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster
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/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically
2012-06-12 9:39 ` Matthieu Moy
@ 2012-06-12 14:48 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2012-06-12 14:48 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Jeff King, git
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-06-12 14:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-11 10:46 [PATCH] api-credential.txt: document that helpers field is filled-in automatically Matthieu Moy
2012-06-11 16:12 ` Junio C Hamano
2012-06-11 17:51 ` Matthieu Moy
2012-06-11 19:08 ` Jeff King
2012-06-12 9:39 ` Matthieu Moy
2012-06-12 14:48 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).