Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

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

 



On 06/12/2015 11:10 PM, Junio C Hamano wrote:
Karthik Nayak <karthik.188@xxxxxxxxx> writes:

The comment in 'ref_sort' hasn't been changed 9f613dd.

Bad grammar?  "hasn't been changed since 9f613dd", perhaps?

Yes! thanks :)


But more importantly, don't just give an abbreviated object name.  I
think "the comment hasn't changed since the for-each-ref command was
originally introduced" is what you meant to say, and it is OK to
append "since 9f613ddd (Add git-for-each-ref: helper for language
bindings, 2006-09-15)" to that sentence as a supporting material.


Ok will do.

Change the comment to reflect changes made in the code since
9f613dd.

What change since 9f613dd do you have in mind, exactly, though?

Well initially the atoms were indexed into used_atom array, which
later was removed. Hence the comment becomes obsolete.


I do not think the fact that this field indexes into used_atom[]
array has ever changed during the life of this implementation.
I see "static const char **used_atom;" in builtin/for-each-ref.c
still in the 'master', and that is the array that holds the atoms
that are used by the end-user request.

So I do not think "The comment was there from the beginning, it
described the initial implementation, the implementation was updated
and the comment has become stale" is a good justification for this
change, as I do not think that is what has happened here.

You may be changing used_atom to something else later in your
series, but then isn't that commit the appropriate place to update
this comment?


But isn't that what happened here, the code was altered but the comment
was left the way it is.

What do you suggest I do?

--
Regards,
Karthik
--
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]