On Sun, Oct 29, 2017 at 1:57 PM, brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > On Sat, Oct 28, 2017 at 09:44:07PM -0400, Eric Sunshine wrote: >> > +#define current_hash the_repository->hash_algo >> >> The all-lowercase name "current_hash" seems likely to conflict with a >> variable name some day; the fact that it is also a #define makes such >> a collision even more worrisome. Although it is retrieving the "hash >> algorithm", when reading the terse name "current_hash", one may >> instead intuitively think it is referring to a hash _value_ (not an >> algorithm). > > I can do CURRENT_HASH_ALGO or CURRENT_HASH instead if you think that's > an improvement. I originally omitted the "algo" portion to keep it > short. I don't have strong feelings about it aside from worrying about a "current_hash" name clash or a reader misunderstanding what it represents. Does "current" need to be in the name? What about HASH_ALGO or REPO_HASH_ALGO? > Alternatively, we could have a current_hash() (or current_hash_algo()) > inline function if people like that better. hash_algo() or repo_hash_algo()?