Subject: Re: [PATCH v20 00/48] Use ref transactions

From: Michael Haggerty <[email-masked]>
Date: Tue, 08 Jul 2014 18:29:07 +0200

On 06/20/2014 04:42 PM, Ronnie Sahlberg wrote:
> This patch series can also be found at
> https://github.com/rsahlberg/git/tree/ref-transactions
> 
> This patch series is based on current master and expands on the transaction
> API. It converts all ref updates, inside refs.c as well as external, to use the
> transaction API for updates. This makes most of the ref updates to become
> atomic when there are failures locking or writing to a ref.
> 
> This version completes the work to convert all ref updates to use transactions.
> Now that all updates are through transactions I will start working on
> cleaning up the reading of refs and to create an api for managing reflogs but
> all that will go in a different patch series.
> 
> Version 20:
>  - Whitespace and style changes suggested by Jun.

I spent most of the day on reviewing this patch series, but now I'm out
of time again.  Here is a summary from my point of view:

Patches 01-19 -- ACK mhagger
Patches 20-42 -- I sent various comments, small to large, concerning
these patches
Patch 43 -- Needs more justification if it is to be acceptable
Patch 44 -- Depends on 43
Patches 45-48 -- I didn't quite get to these, but...

Perhaps it would be more appropriate for the rules about reference name
conflicts to be enforced by the backend, since it is the limitations of
the current backend that impose the restrictions.  Would that make sense?

On the other hand, removing the restrictions isn't simply a matter of
picking a different backend, because all Git repositories have to be
able to interact with each other.

So, I don't yet have a considered opinion on the matter.

I think it would be good to try to merge the first part of this patch
series to lock in some progress while we continue iterating on the
remainder.  I'm satisfied that it is all going in the right direction
and I am thankful to Ronnie for pushing it forward.  But handling
48-patch series is very daunting and I would welcome a split.

I'm not sure whether patches 01-19 are necessarily the right split
between merge-now/iterate-more; it is more or less an accident that I
stopped after patch 19 on an earlier review.  Maybe Ronnie could propose
a logical subset of the commits as being ready to be merged to next in
the nearish term?

Michael

-- 
Michael Haggerty
[email-masked]