On Sat, 25 Mar 2023 11:18:32 GMT, Lance Andersen <lan...@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