Re: [PATCH 0/3] Extract memory pool logic into reusable component

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 21, 2018, 19:27 UTC
Message-ID: <xmqq8talp6u9.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180321164152.204869-1-jamill@microsoft.com>

jameson.miller81@gmail.com writes:

> This patch series extracts the memory pool implementation, currently
> used by fast-import, into a generalized component. This memory pool
> can then be generally used by any component that needs a pool of
> memory.

The way this series is organized is quite reviewer hostile.

Step #2 that adds bunch of new code without introducing any caller nor adjusting existing code means the readers will have no idea how the new code relates to the existing code in fast-import.c (e.g. "Is it just a rename of some type, structure fields, function names etc., and moving the lines without no other change?" "Is the new code more capable than fast-import's one and does step #3 just makes the existing fast-import callers use just a subset of the feature?").

Either case, if I were doing this (by the way, I am not in particular an expert in this area), I probably would make existing memtool implementation more generic (e.g. renaming, feature generalization, etc.) and adjust existing callers while the code is still in its original place in one step, and then in a separate step move the resulting reusable bits to a new file wilt a new header.