Re: [PATCH v3 03/10] trailer: unify trailer formatting machinery

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

Linus Arver <linusa@google.com> writes:

>> In any case, the updated code does try to call unfold_value() in
>> format_trailers() on "val" that has already been unfolded in
>> parse_trailers().
>
> Correct. But this was already the case before this series. IOW the
> existing code assumes that this function is idempotent: we call
> unfold_value() in parse_trailers() and again in format_trailer_info().

But not in format_trailers(), which is the theme of this step.  In
other words, the behaviour before and after this step of the
function are not the same (modulo that the new version stores the
output in a strbuf), and as the way the changes are presented here,
it is almost impossible to make sure that we are not introducing
regressions, without making such assumptions like "unfold_value() is
idempotent and we already rely on that fact elsewhere in the code".

It is not just "unfold", which was the first thing I happened to
notice, by the way.  There are many more lines in the new lines of
that function, doing things that the original version did not appear
to do, or doing in very different ways.

> I could just grow this series with another ~22 patches to include those
> additional refactors, but I am hesitant about doing so, simply due to
> the sheer number of them.

I actually do not mind at all if you started with a preliminary
clean-up series, and stopped the first batch somewhere in the middle
of the 20+ patches before even reaching any of these 10 patches we
see here, if that gives us a readable set of bite-sized changes that
prepare a solid foundation to rebuild things on top.  I am having a
feeling that not even a single person has reviewed them on list even
though we are already at the third iteration, which is quite
frustrating (and I would imagine that it would be frustrating for
you, too), and I suspect that the step like [v3 03/10] that makes
too large a change with too little explanation (and perhaps a bit of
"trust me, this does not change the behaviour") is one contributing
factor why people are afraid of touching it.