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

Reply via email to