Re: [PATCH v2 19/19] Rename struct git_attr_check to git_attr_value

Thanks for the good feedback.

On 08/02/2011 05:46 PM, Junio C Hamano wrote:

Michael Haggerty [off-list ref] writes:

quoted

This described its purpose better, especially when used with
git_allattrs().

You probably meant s/described/describes/ but more importantly does it
really? It is a structure used to probe into the attributes system for the
state of various attributes on a path, and the set of possible states
includes "there is no value" (aka unset), so it feels actively wrong to
call it attr_value and that is why I didn't call it in the first place.

I don't think it is so unusual for a "value" object to be able to
reflect the fact that the value is unset, but I can understand your
point of view too.  I will omit this renaming in the re-roll.

I also think git_all_attrs() (i.e. word-break underscore after "all") is
more in line with the naming throughout the codebase, after looking at
output from

  $ git grep -e _all'[a-z]' --and --not -e alloc -e _all_ -- '*.c'

Other than these, and the earlier comment about the copy&paste done from
git_checkattr (which by the way should probably be "git_check_attr"), it
seems that the series mostly consist of good clean-ups and an addition of
a new and (probably) useful feature that is straightforward. Nice.

I thought the name was awkward, too, but I chose it to be consistent
with git_checkattr().  So in the re-roll I will happily rename both
functions.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/