Re: [PATCH 2/3] sequencer: use run_commit_hook()

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Oct 14, 2019, 22:14 UTC
Message-ID: <nycvar.QRO.7.76.6.1910150013010.46@tvgsbejvaqbjf.bet>
In-Reply-To: <7a56faf1-a0e3-98ad-bdfe-e9ba0b97b5c2@gmail.com>

Hi Phillip,

On Mon, 14 Oct 2019, Phillip Wood wrote:

Show 21 quoted lines

> Hi Dscho & Junio
>
> On 11/10/2019 05:24, Junio C Hamano wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> >>>  builtin/commit.c | 22 ----------------------
> >>>  commit.h         |  3 ---
> >>>  sequencer.c      | 45 ++++++++++++++++++++++++++++++++++-----------
> >>>  sequencer.h      |  2 ++
> >>>  4 files changed, 36 insertions(+), 36 deletions(-)
> >>
> >> Hmm. I would have thought that `commit.c` would be a more logical home
> >> for that function (and that the declaration could remain in `commit.h`)?
> >
> > Good correction.
>
> There are some other public commit related functions in sequencer.c -
> print_commit_summary(), commit_post_rewrite(), rest_is_empty(),
> cleanup_message(), message_is_empty(), template_untouched(),
> update_head_with_reflog() . Would you like to see them moved to commit.c
> (probably as a separate series)?

I don't think that it is necessary to move any of those functions out of their existing habitat just yet. While I haven't looked more closely which of these functions are specific to the sequencer and which are more generic, I would argue that moving any of them is outside of the goals of your patch series.

Thanks, Dscho