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

[Karthik Nayak <karthik.188@gmail.com>]

On 06/12/2015 11:10 PM, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> 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