From: Junio C Hamano Date: Fri, 12 Jun 2015 17:40:50 GMT Subject: Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort Message-ID: In-Reply-To: <1434039003-10928-3-git-send-email-karthik.188@gmail.com> Karthik Nayak writes: > The comment in 'ref_sort' hasn't been changed 9f613dd. Bad grammar? "hasn't been changed since 9f613dd", perhaps? 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. > Change the comment to reflect changes made in the code since > 9f613dd. What change since 9f613dd do you have in mind, exactly, though? 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? > Mentored-by: Christian Couder > Mentored-by: Matthieu Moy > Signed-off-by: Karthik Nayak > --- > builtin/for-each-ref.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c > index 0dd2df2..bfad03f 100644 > --- a/builtin/for-each-ref.c > +++ b/builtin/for-each-ref.c > @@ -27,7 +27,7 @@ struct atom_value { > > struct ref_sort { > struct ref_sort *next; > - int atom; /* index into used_atom array */ > + int atom; /* index into 'struct atom_value *' array */ > unsigned reverse : 1; > };