Re: [PATCH v2 03/12] struct ref_update: move "have_old" into "flags"

> Stefan Beller <sbeller@google.com> writes:
> 
>> On Thu, Feb 12, 2015 at 3:12 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>>> -       int flags; /* REF_NODEREF? */
>>> -       int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
>>> +       /*
>>> +        * One or more of REF_HAVE_OLD, REF_NODEREF,
>>> +        * REF_DELETING, and REF_IS_PRUNING:
>>> +        */
>>> +       int flags;
>>
>> Nit:
>> I'd find it more readable if it would be:
>>     /*
>>      * One or more of
>>      * REF_HAVE_OLD,
>>      * REF_NODEREF,
>>      * REF_DELETING,
>>      * REF_IS_PRUNING:
>>      * whose definition is found at the top of this file.
>>      */
> 
> I wouldn't do either, though, as you would have to keep repeating
> yourself here and over there.  Wouldn't it be sufficient to:
> 
>  - Have a header that says "these are bits meant to go to struct
>    ref_update.flags" at the beginning of series of #define's;
> 
>  - Say "ref_update.flags bits are defined above" here.  The phrasing
>    can be "One or more of REF_HAVE_OLD, etc. defined above." as long
>    as it is clear that this is not meant to be an exhausitive list.