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?
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 <christian.couder@xxxxxxxxx>
> Mentored-by: Matthieu Moy <matthieu.moy@xxxxxxxxxxxxxxx>
> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> ---
> 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;
> };
--
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