RFR: 6968351: httpserver clashes with delayed TCP ACKs for low Content-Length [v17]

[robert engels]

On Thu, 25 Apr 2024 12:58:36 GMT, robert engels <duke at openjdk.org> wrote:

>> We have been using these newer constructs whenever a new test gets added, but we don't update all tests in one go for such changes. If/when old tests are updated for some bug fix relevant to that test, depending on the complexity we either decide to let them stay as-is or update them to use these newer constructs.
>
> Personally, I think that is not the correct choice. I think consistency is more important - especially in a code base this large. 
> 
> But I’ll do whatever you want. This is getting a bit frustrating. The PR review is 1% content and 99% personal nits. 
> 
> This is certainly a different approach than Google uses.

To clarify the above. The standard should be - does this PR improve the code base. After which, if someone wishes to create a refactoring PR that addresses other issues - go for it. But don’t drag out someone’s already considerable effort in diagnosing and fixing the problem to satisfy other goals.

Clearly there is a lot of test code that could use cleanup - but this is not the way to go about doing that - holding up important fixes as an example. 

Most developers are going to look to existing code when implementing changes - if you have a “bunch of stuff in your head” you’d really like it to adhere to it is going to be a frustrating process and people won’t bother.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579479267