Re: [RFC PATCH v2 00/22] Add blame to libgit

[Junio C Hamano <gitster@pobox.com>]

Jeffrey Smith <whydoubt@gmail.com> writes:

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