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

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Matthieu Moy <hidden>
Date: 2016-06-15 23:05:43

Karthik Nayak [off-list ref] 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.

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.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

From: Karthik Nayak <hidden>
Date: 2016-06-15 23:05:44

On Thu, Jul 9, 2015 at 6:59 PM, Matthieu Moy
[off-list ref] wrote:
Karthik Nayak [off-list ref] writes:
quoted
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.
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

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

From: Karthik Nayak <hidden>
Date: 2016-06-15 23:05:44

On Fri, Jul 10, 2015 at 4:22 PM, Karthik Nayak [off-list ref] wrote:
On Thu, Jul 9, 2015 at 6:59 PM, Matthieu Moy
[off-list ref] wrote:
quoted
Karthik Nayak [off-list ref] writes:
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help