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

From: Jeff King <peff@peff.net>
Date: Jun 11, 2012, 19:08 UTC
Message-ID: <20120611190811.GB20889@sigill.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:

Show 12 quoted lines

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

Show 12 quoted lines

> 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