Re: [PATCH v3 18/44] refs: move transaction functions into common code

[Michael Haggerty <mhagger@alum.mit.edu>]

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

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/279286
[2] https://github.com/mhagger/git

-- 
Michael Haggerty
mhagger@alum.mit.edu