On Fri, Jun 23, 2017 at 12:35 PM, Jeff King <peff@xxxxxxxx> wrote: > On Fri, Jun 23, 2017 at 09:01:37AM +0200, Michael Haggerty wrote: > >> Now that the interface between `files_ref_store` and >> `packed_ref_store` is relatively narrow, move the latter into a new >> module, "refs/packed-backend.h" and "refs/packed-backend.c". It still >> doesn't quite implement the `ref_store` interface, but it will soon. >> >> This commit moves code around and adjusts its visibility, but doesn't >> change anything. > > Looks good. Stefan will be happy to know that I used --color-moved to > look at it. ;) Hah! As a follow up on that, let's perform a user survey :-P * How was your review experience? * Were you annoyed by the default colors or mode? (That is best expressed as a patch, as it seems like bikeshedding to me, but I am far from being a UX expert ;) Just today I thought about that further: While reviewing is one thing (which I do a lot), how can we make this work with merging changes? I think the file rename detection is acknowledged by the merge code, such that a plain file rename and a patch to said file is easy on the maintainer, but we would want that for smaller code movements, too. Let's take this patch as an example, if someone were to find a bug in one of the moved functions, they would send a fix based on the function in refs/files-backend.c, such that it can easily be merged down to maint, but when merging it forward with this, it may clash.
