From: Karthik Nayak Date: Fri, 12 Jun 2015 17:48:16 GMT Subject: Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort Message-ID: <557B1B60.9020002@gmail.com> In-Reply-To: On 06/12/2015 11:10 PM, Junio C Hamano wrote: > Karthik Nayak 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