Hm, I really thought I had split some of those out. Patches 4, 5, 17, and 19 could all be reasonably split into rename vs move patches. Patch 18 only does moving. I do not see much benefit to splitting up the 'move xyz to scoreboard' subset (6-12) that way as moving an item to scoreboard is already causing each use to change (from xyz to sb->xyz). On Mon, May 15, 2017 at 7:23 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > 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...) >> >>