COMPRESS-721 -- avoid reflection when possible in unpack200#764
COMPRESS-721 -- avoid reflection when possible in unpack200#764tballison wants to merge 7 commits intoapache:masterfrom
Conversation
|
I added unit tests that fail when building without the |
garydgregory
left a comment
There was a problem hiding this comment.
Hello @tballison
-1: These tests don't test anything new. They always pass, whether or not the changes to main are applied, inviting a regression to whatever you think you're fixing.
|
@garydgregory LOL...y, I added those when I was manually testing by removing the java >=17 profile in the pom.xml. However, as you point out, the build already fails spectacularly with existing tests when that is removed because of exactly this issue. |
|
As a side note, the more aggressive fix is to get rid of reflection entirely. I chose to modify the existing code as little as possible on this PR, but happy to go bigger if desired. |
|
Sorry for the noise, |
|
Hello @tballison There might be a way to do this without reflection, if so please provide it 😁 Otherwise, these untested changes are Just asking for a regression in the future... |
|
fix incoming, again apologies for the noise. |
Thanks for your contribution to Apache Commons! Your help is appreciated!
Before you push a pull request, review this list:
Claude Opus 4.6 significantly
mvn; that'smvnon the command line by itself.