William A. Rowe, Jr. wrote:
I see lots of comments on the code, but the comments are summarised as "the cache is fine as it is". It isn't. If it was fine, key users of network caching wouldn't be standing up saying they're using something else.I concur, but the history becomes a nightmare. Let's back out the vetoed efforts and work up -clean- patches to apply that solve one issue each, and don't raise the objections again?
Currently I am aware of only one outstanding -1, and it came without any technical justification, and so meant "I disagree". This most recent objection was based on the assumption that r450105 still stood, when this is no longer the case. Unfortunately r450105 was not in itself a clean patch, and could not be backed out cleanly like the split buckets patch. If you look at r467655, a large chunk of the patch is simply the deletion of the contentious copy_body() function.
Joe also -1'd the split buckets patch from yesterday, as it duplicated the native behaviour of ap_bucket_read(). This was backed out cleanly late yesterday in preparation for today's fix, based on Joe's suggestion on how to solve the problem.
I understand the desire to make incremental progress, but patches of patches of patches make the overall history hard to follow, and the net results harder to review.
A significant amount of work has been done by Niklas to break up a monolithic change that what was never designed to be a series of patches into a series of patches, and to be honest I think we have the best we could have at this point. If we start backing out and reapplying again, the history will be even harder to follow.
Regards, Graham --
smime.p7s
Description: S/MIME Cryptographic Signature
