# [PATCH] api-credential.txt: document that helpers field is filled-in automatically 6 messages from 2012-06-11 to 2012-06-12. Participants: Matthieu Moy, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/30764 ## Matthieu Moy, 2012-06-11 10:46 Subject: [PATCH] api-credential.txt: document that helpers field is filled-in automatically Message-ID: <1339411574-22998-1-git-send-email-Matthieu.Moy@imag.fr> URL: https://gitlist.dev/e/1339411574-22998-1-git-send-email-Matthieu.Moy%40imag.fr ``` It was unclear whether the field was to be specified by the user of the API. Signed-off-by: Matthieu Moy --- 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 ``` ## Junio C Hamano, 2012-06-11 16:12 Subject: Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically Message-ID: <7vzk897rqi.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzk897rqi.fsf%40alter.siamese.dyndns.org In-Reply-To: <1339411574-22998-1-git-send-email-Matthieu.Moy@imag.fr> ``` Matthieu Moy writes: > It was unclear whether the field was to be specified by the user of the > API. > > Signed-off-by: Matthieu Moy > --- > > 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`:: ``` ## Matthieu Moy, 2012-06-11 17:51 Subject: [PATCH] api-credential.txt: document that helpers field is filled-in automatically Message-ID: <1339437107-25909-1-git-send-email-Matthieu.Moy@imag.fr> URL: https://gitlist.dev/e/1339437107-25909-1-git-send-email-Matthieu.Moy%40imag.fr In-Reply-To: <7vzk897rqi.fsf@alter.siamese.dyndns.org> ``` It was unclear whether the field was to be specified by the user of the API. Signed-off-by: Matthieu Moy --- > 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 ``` ## Jeff King, 2012-06-11 19:08 Subject: Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically Message-ID: <20120611190811.GB20889@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120611190811.GB20889%40sigill.intra.peff.net In-Reply-To: <1339437107-25909-1-git-send-email-Matthieu.Moy@imag.fr> ``` 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 > --- > > 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 ``` ## Matthieu Moy, 2012-06-12 09:39 Subject: Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically Message-ID: URL: https://gitlist.dev/e/vpq7gvcx40y.fsf%40bauges.imag.fr In-Reply-To: <20120611190811.GB20889@sigill.intra.peff.net> ``` Jeff King 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 >> --- >> > 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/ ``` ## Junio C Hamano, 2012-06-12 14:48 Subject: Re: [PATCH] api-credential.txt: document that helpers field is filled-in automatically Message-ID: <7vehpk4mde.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vehpk4mde.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Matthieu Moy 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. ```