On Tue, 2015-10-13 at 18:21 -0400, David Turner wrote: > On Tue, 2015-10-13 at 13:29 +0200, Michael Haggerty wrote: > > I reviewed the patches up to here pretty carefully, and aside from the > > comments I already sent, they look good. > > > > I like the new approach where the ref_transaction-building code is > > shared across backends. > > > > It seems to me that a good breaking point for the first batch of patches > > would be here, just before you start creating the VTABLE in commit > > [19/44]. The patches before this point all have to do with moving code > > around and a little bit of light refactoring. They cause a lot of code > > churn that will soon conflict with other people's work (e.g., [1]), but > > I think they are pretty uncontroversial. > > > > After this you start making a few important design decisions that > > *could* be controversial. Therefore, by making a cut, you can maximize > > the chance that the earlier patches can be merged to master relatively > > quickly, after which the cross section for future conflicts will be much > > smaller. > > > > Ideally, you would include a few later patches in the "pre-VTABLE" patch > > series. It looks like the following patches also mostly have to do with > > moving code around, and would fit logically with the "pre-VTABLE" changes: > > > > [24] refs.c: move refname_is_safe to the common code > > [25] refs.h: document make refname_is_safe and add it to header > > [26] refs.c: move copy_msg to the common code > > [27] refs.c: move peel_object to the common code > > [28] refs.c: move should_autocreate_reflog to common code > > [32] initdb: move safe_create_dir into common code > > [36] refs: make files_log_ref_write functions public > > [37] refs: break out a ref conflict check > > > > I tried rebasing those commits on top of your patch 18 and it wasn't too > > bad. The result is on branch "refs-backend-pre-vtable" on my GitHub repo > > [2], including my suggested changes to those eight patches (which > > therefore became seven because I squashed the first two together). > > Thanks. I started from that, and made the changes that you suggested > in reviews that were not yet in there. > > I also added Jeff's extension patch, since it seems uncontroversial to > me, and since we'll need it sooner or later anyway. > > I put the result on my github at: > https://github.com/dturner-tw/git > on the refs-backend-pre-vtable branch While rebasing the rest of the series on this, I noticed an issue in one of the patches, which I have now fixed. I re-pushed. -- 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