git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brandon Williams <bmwill@google.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] pack-protocol: mention and point to docs for protocol v2
Date: Tue, 24 Jul 2018 11:19:13 -0700	[thread overview]
Message-ID: <20180724181913.GA225275@google.com> (raw)
In-Reply-To: <20180724045233.GB208393@aiede.svl.corp.google.com>

On 07/23, Jonathan Nieder wrote:
> Hi,
> 
> Brandon Williams wrote:
> 
> > --- a/Documentation/technical/pack-protocol.txt
> > +++ b/Documentation/technical/pack-protocol.txt
> > @@ -50,7 +50,8 @@ Each Extra Parameter takes the form of `<key>=<value>` or `<key>`.
> >  
> >  Servers that receive any such Extra Parameters MUST ignore all
> >  unrecognized keys. Currently, the only Extra Parameter recognized is
> > -"version=1".
> > +"version" with a vlue of '1' or '2'.  See protocol-v2.txt for more
> 
> value?

yep, missed a letter.

> 
> > +information on protocol version 2.
> 
> Thanks.  Some thoughts on other parts of this document that may need
> updating:
> 
> - the whole document assumes that 0 and 1 are the only protocol
>   versions.  E.g. the discussion of the version number line in the
>   response when "version=1" is sent as an Extra Paramter should probably
>   apply to version 2, too.
> 
> - because the document was written before protocol v2, it describes the
>   more complicated v1 that many readers shouldn't have to care about
> 
> - there is no one document that describes v2 in a self contained way,
>   since protocol-v2.txt makes reference to protocol v1.
> 
> - the description of pkt-line format in protocol-common.txt is missing
>   a discussion of delim-pkt.
> 
> Not about this patch, but I wonder if an organization along the
> following lines would make sense?
> 
>  1. Rename pack-protocol.txt to protocol-v1.txt.  Rename
>     protocol-v2.txt to pack-protocol.txt.
> 
>  2. Make pack-protocol.txt self-contained, and remove any redundant
>     sections from protocol-v1.txt.
> 
>  3. Add a new protocol-v2.txt that briefly describes the benefits and
>     highlights of protocol v2, referring to pack-protocol.txt for
>     details.
> 
> That way, newcomers of the future could read pack-protocol.txt and
> quickly glean the main protocol in (then) current use.
> 
> What do you think?

I dislike the idea of renaming protocol-v2.txt to pack-protocol.txt.  I
agree that we should probably have protocol-v1 broken out into its own
file, taking the parts from pack-protocol.txt, but what really should
happen is that pack-protocol.txt could describe the basics of the wire
protocol (pkt-lines, the format of the various transports, etc) and then
refer to the protocol-v{1,2}.txt documents themselves.

-- 
Brandon Williams

  reply	other threads:[~2018-07-24 18:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-23 17:48 [PATCH] pack-protocol: mention and point to docs for protocol v2 Brandon Williams
2018-07-24  4:52 ` Jonathan Nieder
2018-07-24 18:19   ` Brandon Williams [this message]
2018-07-24 19:57     ` Junio C Hamano
2018-07-24 20:47       ` Brandon Williams
2018-07-24 22:19         ` Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180724181913.GA225275@google.com \
    --to=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).