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 <gits...@pobox.com> wrote:
> Jeffrey Smith <whydo...@gmail.com> writes:
>
>> On Mon, May 15, 2017 at 4:24 AM, Junio C Hamano <gits...@pobox.com> wrote:
>>> Jeff Smith <whydo...@gmail.com> 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...)
>>
>>

Reply via email to