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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 10, 2015 at 4:22 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> On Thu, Jul 9, 2015 at 6:59 PM, Matthieu Moy
> <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
>> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>>
>>> Add support to sort by version using the "v:refname" and
>>> "version:refname" option. This is achieved by using the
>>> 'version_cmp()' function as the comparing function for qsort.
>>
>> You should elaborate on why you need this. Given the context, I can
>> guess that you will need this to implement tag, but for example I first
>> wondered why you needed both version: and v:, but I guess it comes from
>> the fact that 'git tag --sort' can take version:refname or v:refname.
>>
>> I think this deserves a test and documentation in for-each-ref.txt.
>
> I'll add it to "for-each-ref.txt" documentation.
> About the tests, there are already tests for the same in git-tag.txt and
> that's the only reason I did not repeat the tests in for-each-ref.
>

But since the porting is in a later commit, will add tests to for-each-ref.

>>
>> As-is, the code is a bit hard to understand. I first saw you were
>> allowing
>>
>>   git for-each-ref --format '%(version:refname)'
>>
>> (which you are, but only as a side effect), and then understood that
>> this was also allowing
>>
>>   git for-each-ref --sort version:refname
>>
>> A test would have shown me this immediately. Some hints in the commit
>> message would clearly have helped too.
>>
>
> Most of the sorting options have side effects in "--format", but yeah will add
> details in the commit message.
>
> --
> Regards,
> Karthik Nayak



-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]