RFR: 8304013: Add a fast, non-manual alternative to test/jdk/java/util/zip/ZipFile/TestTooManyEntries [v3]
Lance Andersen
lancea at openjdk.org
Sat Mar 25 11:58:30 UTC 2023
On Sat, 25 Mar 2023 11:18:32 GMT, Lance Andersen <lancea at openjdk.org> wrote:
>> Thanks, I've added `@throws IOException if an error occurs` to all methods throwing `IOException`.
>>
>> For the record, let me state my personal (rather strong) opinion:
>>
>> I think this a pretty crazy level of boilerplate noise for a test and that it affects readability negatively. My IDE (IntelliJ) does not complain about this at all. Any IDE which complains should be fixed or configured to not complain. Changing Javadoc to complain about this in a scope like this would be a poor decision.
>>
>> I think the PR is ready to be sponsored after this.
>
> I hear you but the option is not to use javadoc comments and use block comments :-)
>
> I think we can agree to disagree on impacting readability ;-) this comes down to personal preferences
>
> Again thank you for making the change.
> I think the PR is ready to be sponsored after this.
Please see my comment regarding the end of central directory record.
I would prefer to tweak that in a fashion similar to what I indicated as I thought your original version was clearer. I understand what Martin was indicating of the use of EOC and that could have been addressed by adding _(EOC)_ or _ENDHDR_ after _end of central directory record_ to make it clearer
I will leave it up to you as to whether you want to make the change but it would be clearer as I think we took a small step backwards.
Either way you should not need a sponsor and should be good to integrated when you are ready
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12231#discussion_r1148353953
More information about the core-libs-dev
mailing list