Re: [RFC/WIP PATCH 08/11] transport: connect_setup appends protocol version number

From: Stefan Beller <sbeller@google.com>
Date: May 26, 2015, 22:31 UTC
Message-ID: <CAGZ79kY=hZ=6pUbpVHUW8rU3AEMx2TwkBYDsDsZ+bPH_QnsYzA@mail.gmail.com>
In-Reply-To: <xmqq1ti3j7ym.fsf@gitster.dls.corp.google.com>

On Tue, May 26, 2015 at 3:21 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> +     if (transport->smart_options
>> +         && transport->smart_options->transport_version) {
>> +             buf = xmalloc(strlen(remote_program) + 12);
>> +             sprintf(buf, "%s-%d", remote_program,
>> +                     transport->smart_options->transport_version);
>> +             remote_program = buf;
>> +     }
>
> Bikeshedding: so the versioning scheme is that the current one is
> zero, and the next one is two?  I would have expected that there
> would be something like

I think currently we have version 1. But we don't advertise it, so I'll call it 0 (the default or unadvertised or however you name it. 0 as in "unsure" maybe)

I thought about future proofing this version a bit. Say version 2 is bad because I don't have the experience of 10 years Git nor of maintaining large projects and you want to make a version 3 soon. And this would support that just fine.

The meaning being: Any version except 0 should have a dedicated extension -${version} The 0 is left out for backwards compatibility.

So in a later patch where we want to introduce force-using of old versions you could configure upload-pack to be explicit upload-pack-1. The upload-pack-1 version is not yet there with this series though.

Show 6 quoted lines

>
>         if (...->version < 2) {
>                 ... append "-%d" ...
>         }
>
> involved.

Oh! I see here you would count the current one as 1, which has no number extension, and any further would have a -${version}. That would transport the intention much better I guess.