Feel free to filter out unworthy commits.

Comment From: snicoll

Thanks for the PR @quaff

Feel free to filter out unworthy commits.

I might do that rather aggressively as the changes are large and we usually prefer to do such stylistic changes to the code base ourselves when time permits. Things like using Map.computeIfAbsent and replacing code with a lambda can have subtle side effects so I am probably going to remove those at least.

That being said, now that your IDE has done the work with you, feel free to resubmit those for test classes only as it is less contentious.

Comment From: snicoll

Here's a summary of what I've done, you can see more about it in the polish commits:

  • I've dropped bd353f1 as we don't have a specific requirements to suppress warnings like that in tests. I know it's mixed at the moment but I'd rather not do any change until there's a policy in place.
  • In general, please re-read before submitting, especially for large commits like that as restoring the format and minimizing noises is really not a great way of spending our time. 7cfff404 and a6e87b40 shows that.
  • I didn't find the move to some text blocks to be much readable, actually. So I've reverted them
  • The move to Objects.equals could have used a re-read, especially since the same pattern of a double negation was introduced several times (adcf236a)
  • The try/resources had several note where the thrown exception was ignored on purpose. Moving to try/resource means that this exception is now thrown. These had explicit note that you seem to have ignored. Again, with a large change like that, it takes significant time to review it does not introduce a regression. And that's not something to expect from a "polish" contribution. I've reverted the commit.
  • I've reverted the commit that moves to lambda as it was too large and I had already seen a few that didn't improve readability.

Comment From: snicoll

@JanStureNielsen this PR is closed. Please read the comment above yours.