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

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

Jeff Smith <whydoubt@gmail.com> writes:

> Jeff Smith (10):
>   Remove unneeded dependency on blob.h from blame
>   Move textconv_object to be with other textconv methods
>   Add some missing definitions to header files
>   Remove unused parameter from get_origin()
>   Split blame origin into its own file
>   Move fake_working_tree_commit() to lib
>   Break out scoreboard a little better
>   Split blame scoreboard into its own file
>   Break out scoreboard init and setup
>   Move scoreboard init and setup to lib

Make sure these shortlog entries, when eventually appear along with
other people's changes, let readers know what these are about
easily, and formatted similarly to others'.  Write the "<area>: "
prefix in front is a good pratice to help that, e.g.

    blame: remove unneeded dependency on blob.h

In general I am OK with the idea of splitting builtin/blame.c into
the blame Porcelain part (i.e. cmd_blame()) and some form of
reusable "blame library".

However, you'd need to be cafreful about names.  Are contents of
"commit-fake", "origin" and "scoreboard" generally useful outside
the context of running a "blame"?  They are not good filenames if
saying "origin.c" in the context of the entire Git codebase, anybody
would understand that it is about "the origin" concept used by the
blame engine (similarly for other new files).  I would imagine that
having blame.[ch] at the toplevel instead of creating that many
"blame library" files would be a good way to go (mirrors how other
parts of the system is laid out, e.g. diff.[ch] and diff-*.[ch] are
library-ish files, while builtin/diff*.c implements the Porcelain).

Have you renamed the names of structures, functions and variables
that used to be private to builtin/blame.c and now shared between
builtin/blame.c and these new files so that they are specific enough
and are obvious that they are about blame infrastructure?  Names
that were perfectly acceptable and appropriate as private ones
inside a subsystem are often too vague or generic when exposed to
outside world as part of the library-ish code.