Re: [PATCH v8 34/44] refs.c: make prune_ref use a transaction to delete the ref

From: Ronnie Sahlberg <sahlberg@google.com>
Date: May 22, 2014, 16:56 UTC
Message-ID: <CAL=YDWkGWwBaNqACS=wYj-kFg2FLddp8n4U=LrUdR0-4sDh9_g@mail.gmail.com>
In-Reply-To: <20140521230139.GJ12314@google.com>

Added a comment that any flags >=0x100 are reserved for internal use.

On Wed, May 21, 2014 at 4:01 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 25 quoted lines

> Ronnie Sahlberg wrote:
>
>> Change prune_ref to delete the ref using a ref transaction. To do this we also
>> need to add a new flag REF_ISPRUNING that will tell the transaction that we
>> do not want to delete this ref from the packed refs.
>
> Interesting.  Since the flag is per ref update, it even would allow
> deleting some refs and pruning others in the same transaction.  Makes
> sense.
>
> Looks like this doesn't batch up multiple ref-prunings into a single
> transaction.  Makes sense (it would just make the period while refs
> are locked longer without having any real benefit).
>
> [...]
>> +#define REF_ISPRUNING        0x0100
>
> Can this conflict with bit values declared elsewhere some day?  It
> would be more comfortable if refs.h also had a note about bits >=
> 0x100 being reserved for private use.
>
> The rest of the patch looks good.
>
> Thanks,
> Jonathan