Re: [PATCH v2 00/25] Prepare to separate out a packed_ref_store
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 24, 2017, 02:45 UTC
- Message-ID
- <xmqq7f17ht0x.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <cover.1495460199.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 5 quoted lines
> * Since v1, branch `bc/object-id` has been merged to `next`, and it > has lots of conflicts with these changes. So I rebased this branch > onto a merge of `master` and `bc/object-id`. (I hope this makes > Junio's job easier.) This unfortunately causes a bit of tbdiff noise > between v1 and v2.
Heh, that gives me an excellent excuse to procrastinate, as I am planning to merge that topic to 'master' by the end of this week ;-)
Jokes aside...
Show 19 quoted lines
> * Patch [01/25]: in t3600, register the `test_when_finished` command
> before executing `chmod a-w`.
>
> * Patch [04/25] (new patch): convert a few `die("internal error: ...")`
> to `die("BUG: ...")`.
>
> * Patch [05/25]: Use `strlen()` rather than `memchr()` to check the
> trim length, and `die()` rather than skipping if it is longer than
> the reference name.
>
> * Patch [08/25]: Name the log message arguments `msg` for consistency
> with existing code.
>
> * Patch [10/25]: Rename the new member from `packlock` to
> `packed_refs_lock`.
>
> * Patch [13/25] (new patch): Move the check for valid
> `transaction->state` from `files_transaction_commit()` to
> `ref_transaction_commit()`.All of these feel familiar ;-)
Show 16 quoted lines
> * Patch [14/25]: > > * Add more sanity checks of `transaction->state`. > > * Don't add `ref_transaction_finish()` to the public API. Instead, > teach `ref_transaction_commit()` to do the right thing whether or > not `ref_transaction_prepare()` has been called. > > * Add and improve docstrings. > > * Allow `ref_transaction_abort()` to be called even before > `ref_transaction_prepare()` (in which case it just calls > `ref_transaction_free()`). > > * Lots of improvements to commit messages and comments, mostly to > clarify points that reviewers asked about.
Overall this looked like quite a quality work to me.
Show 5 quoted lines
> These changes (along with the merge commit that they are based on) are > also available as branch `packed-ref-store-prep` in my GitHub fork > [2]. If you'd like to see a preview of the rest of the changes (which > works but is not yet polished), checkout the `mmap-packed-refs` branch > from the same place.
Thanks.