Re: [PATCH 2/5] Document details of transport function APIs

[Junio C Hamano <gitster@pobox.com>]

Daniel Barkalow <barkalow@iabervon.org> writes:

> In particular, explain which of the fields of struct ref is used for
> what purpose in the input to and output from each function.
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  transport.h |   38 ++++++++++++++++++++++++++++++++++++++
>  1 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/transport.h b/transport.h
> index 489e96a..2e1650a 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -18,11 +18,49 @@ struct transport {
>  	int (*set_option)(struct transport *connection, const char *name,
>  			  const char *value);
>  
> +	/**
> +	 * Returns a list of the remote side's refs. In order to allow
> +	 * the transport to try to share connections, for_push is a
> +	 * hint as to whether the ultimate operation is a push or a fetch.
> +	 *

It is unclear what this "hint" is intended to be used for, what the
transport is and isn't allowed to use it for, and what existing transports
typically use it for.

> +	/**
> +	 * Push the objects and refs. Send the necessary objects, and
> +	 * then tell the remote side to update each ref in the list
> +	 * from old_sha1 to new_sha1.
> +	 *
> +	 * Where possible, set the status for each ref appropriately.
> +	 *
> +	 * If, in the process, the transport determines that the
> +	 * remote side actually responded to the push by updating the
> +	 * ref to a different value, the transport should modify the
> +	 * new_sha1 in the ref. (Note that this is a matter of the
> +	 * remote accepting but rewriting the change, not rejecting it
> +	 * and reporting that a different update had already taken
> +	 * place)
> +	 **/

It this even a sane thing to allow?  How would it interact with the
"pretend we immediately turned around and fetched them into the remote
tracking branches" local updates we usually do?

>  	int (*push_refs)(struct transport *transport, struct ref *refs, int flags);
>  	int (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);
>  
> +	/** get_refs_list(), fetch(), and push_refs() can keep
> +	 * resources (such as a connection) reserved for futher
> +	 * use. disconnect() releases these resources.
> +	 **/
>  	int (*disconnect)(struct transport *connection);
>  	char *pack_lockfile;
>  	signed verbose : 2;

It is just a style thing, but all of our multi-line comments are

   /*
    * of
    * this
    * form
    */

and these new comments are formatted slightly differently with double
asterisks on only the first and the last lines.  In addition, th last
comment block uses a yet another different style, which is a bit of
eyesore.