Re: [PATCH 1/7] pack-protocol.txt: Add warning about protocol inaccuracies

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 1, 2015, 19:39 UTC
Message-ID: <20150701193949.GC4865@google.com>
In-Reply-To: <1435774099-21260-2-git-send-email-dborowitz@google.com>

Hi,

Dave Borowitz wrote:

Show 12 quoted lines

> --- a/Documentation/technical/pack-protocol.txt
> +++ b/Documentation/technical/pack-protocol.txt
> @@ -14,6 +14,17 @@ data.  The protocol functions to have a server tell a client what is
>  currently on the server, then for the two to negotiate the smallest amount
>  of data to send in order to fully update one or the other.
>  
> +Notes to Implementors
> +---------------------
> +
> +WARNING: This document is a work in progress. Some of the protocol
> +specifications below may be incomplete or inaccurate. When in doubt,
> +refer to the C code.

If we include this warning, can it also say to contact git@vger.kernel.org for inaccuracies?

Otherwise it is easy to misread as "Some of this document may be inaccurate, and we're working on fixing that. Don't bug me --- I already know about the problems --- just be patient!"

I would rather that it would say something like

	Caveat
	------
	This document contains some inaccuracies.  Do not forget to also
	check against the C code, and please contact git@vger.kernel.org
	if you run into any unclear or inaccurate passages in this spec.

> +
> +One particular example is that many of the LFs referenced in the
> +specifications are optional, but may (yet) not be marked as such. If not
> +explicitly marked one way or the other, double-check with the C code.

The 'Reference Discovery' section says:

	Server SHOULD terminate each non-flush line using LF ("\n") terminator;
	client MUST NOT complain if there is no terminator.

Unfortunately that's not explained in a section with broader scope.

Isn't that actually the rule everywhere except for in push certs? The documentation will be easier to use if it says so instead of asking implementers to check the source of all implementations (since interoperability with only one isn't enough).

Thanks, Jonathan