Linus Arver <linusa@xxxxxxxxxx> 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.
