Re: [PATCH v4 05/10] ref-filter: add support to sort by version

From: Junio C Hamano <gitster@pobox.com>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: Git <git@vger.kernel.org>,
	Christian Couder <christian.couder@gmail.com>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Subject: Re: [PATCH v4 05/10] ref-filter: add support to sort by version
Date: Mon, 27 Jul 2015 08:24:22 -0700	[thread overview]
Message-ID: <xmqqvbd5ipy1.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAOLa=ZQ89Vz82ro_V8mTgYKZ0UUgbsbAW6zgSgyiZ1gpwjTOHA@mail.gmail.com> (Karthik Nayak's message of "Sun, 26 Jul 2015 10:37:57 +0530")

Karthik Nayak <karthik.188@gmail.com> writes:

> On Sun, Jul 26, 2015 at 4:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Without looking at the callers, s->version looks like a misdesign
>> that should be updated to use the same cmp_type mechanism?  That
>> would lead to even more obvious construct that is easy to enhance,
>> i.e.
>>
>>         switch (cmp_type) {
>>         case CMP_VERSION:
>>                 ...
>>         case CMP_STRING:
>>                 ...
>>         case CMP_NUMBER:
>>                 ...
>>         }
>>
>> I dunno.
>>
>> Other than that (and the structure of that "format-state" stuff we
>> discussed separately), the series was a pleasant read.
>>
>> Thanks.
>
> That was the previous design, but Duy asked me to do this so
> that we could support all atoms. And I agree with him on this.
>
> http://article.gmane.org/gmane.comp.version-control.git/273888

I am not objecting, but $gmane/273888 does not immediately read, at
least to me, as suggesting using a mechanism different from cmp_type
but a dedicated field s->version.  Puzzled...