On Fri, Feb 18 2022, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> diff --git a/reflog.h b/reflog.h
>> new file mode 100644
>> index 00000000000..e4a8a104f45
>> --- /dev/null
>> +++ b/reflog.h
>> @@ -0,0 +1,49 @@
>> +#ifndef REFLOG_H
>> +#define REFLOG_H
>> +
>> +#include "cache.h"
>> +#include "commit.h"
>> +
>> +/* Remember to update object flag allocation in object.h */
>> +#define INCOMPLETE (1u<<10)
>> +#define STUDYING (1u<<11)
>> +#define REACHABLE (1u<<12)
>
> These names were unique enough back when they were part of internal
> implementation detail inside builtin/reflog.c but it no longer is in
> the scope it is visible. builtin/stash.c (and whatever other things
> that may want to deal with reflogs in the future) is about stash
> entries and wants to have a way to differentiate these symbols from
> others and hint that these are about reflog operation.
>
> Perhaps prefix them with REFLOG_ or something?
>
> Or, do we even NEED to expose these bits outside the implementation
> of reflog.c? If these three constants are used ONLY by reflog.c and
> not by builtin/reflog.c (or builtin/stash.c), then moving them to
> where they are used, i.e. in reflog.c, without exposing them to
> others in reflog.h, would be a far better thing to do. That way,
> they can stay with their original name, without having to add a
> differentiating prefix.
No objection to this, but FWIW the general pattern for these object.h
flags is to use these un-prefixed names:
git grep -A20 'Remember to update object flag allocation' | grep define
To be fair the only one that's really comparable is the revision.h ones,
but those are very non-namespace-y, with names like SEEN, ADDED, SHOWN
etc.
I'd be fine with just leaving these as-is, especially with compilers
warning about macro re-definitions.
