Jeffrey Smith <whydoubt@xxxxxxxxx> writes: > On Mon, May 15, 2017 at 4:24 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Jeff Smith <whydoubt@xxxxxxxxx> writes: >> >>> Rather than duplicate large portions of builtin/blame.c in cgit, it >>> would be better to shift its core functionality into libgit.a. The >>> functionality left in builtin/blame.c mostly relates to terminal >>> presentation. >> >> As I said in my review of 04/22, it is a bit hard/tedious to sift >> the changes to refactoring that actually changes code and pure >> renaming and movement of lines across files with the current >> sequence of the series, so it is very possible that I may have >> missed something, but from a cursory read through the series, from >> the comparison between master and the result of applying all >> patches, and inspection of the resulting builtin/blame.c file, I >> think what this series does is very sensible in general. blame.h >> does not seem to expose anything that is not needed, which is a good >> sign. >> > I did try to keep any unnecessary changes out of the big movement > patches (17-19). Would you prefer I break down 19 further? I had > been holding back because of the added churn that would introduce from > lots of changing function visibility in blame.h and blame.c. To be honest, the "rename symbols while moving" done starting around 04/22 made the series too noisy to read and I had hard time reviewing the later ones like 17-19. I think they share exactly the same issue as 04/22, e.g. 17/22 renames origin_decref to blame_origin_decref while moving the function and some of its callers across files. > > (And if gmail would quit trying to make my messages 'rich text' -- > a.k.a. HTML -- that would be great...) > >