On Tue, 11 Oct 2022 08:48:20 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 80:
>> 
>>> 78:                 """;
>>> 79:         frame = new JFrame("JFileChooser File View NPE test");
>>> 80:         passFailJFrame = new PassFailJFrame("Test Instructions", 
>>> INSTRUCTIONS, 5L, 13, 40);
>> 
>> Few lines are going beyond 80 columns, can you please update them?
>
>> Few lines are going beyond 80 columns, can you please update them?
> 
> I agree, line 80 could be wrapped, it's 92 columns long. At the same, I 
> wouldn't have insisted because it contains the parameters to `PassFailJFrame` 
> which aren't so important to understanding the logic of the test.
> 
> Line 92 with the position is better left untouched, it's not too long (five 
> chars outside of 80-column limit) and it's cleaner unwrapped. You may 
> statically-import `PassFailJFrame.Position` in which case the line would 
> become shorter:
> 
>         PassFailJFrame.positionTestWindow(frame, Position.HORIZONTAL);
> 
> 
> I am for keeping line 61 as it is now even though it doesn't fit the limit by 
> 5 characters. It's still readable. In fact, this `throws` clause could be 
> avoided altogether by restructuring the code. The constructor of 
> `PassFailJFrame` is designed to be called on the main thread, yet Tejesh 
> prefers it this way. I don't think it's worth moving the code around at this 
> point when it's approved already.

Ok.

-------------

PR: https://git.openjdk.org/jdk/pull/10485

Reply via email to