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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 8, 2014 at 9:29 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> 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,

Thanks!


>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 for compatibility I would prefer to keep the same rules for
name conflicts as for the current files implementation.
But we could have a configuration option to disable these checks, with
the caveat that this might mean that some users will
no longer be able to access pull all the branches anymore.


>
>
> 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.

Will do.

>
> 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
> mhagger@xxxxxxxxxxxx
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]