<Swing Dev> REM: Re: [9-client] Review request for bug 7124238 : [TEST_BUG] Font in BasicHTML document is bigger than it should be

[Alexander Zvegintsev]

Hi Shilpi,

   56         test();
   57         f.dispose();


In case of test failure runtime exception will be thrown, thus 
f.dispose() will not be called.
So it would be better to call it from a finally block.

The fix contains mixed tab/spaces indentation(we are using spaces), 
trailing whitespaces,
this will prevent push to openjdk repository.

Also there are a lot of empty lines at the end of file.

Thanks,

Alexander.

On 08/24/2015 02:47 PM, shilpi rastogi wrote:
> Hi all,
>
> Gentle reminder.
>
> Thanks
> Shilpi
> On 8/14/2015 1:23 PM, Alexander Scherbatiy wrote:
>>
>>   The fix looks good to me.
>>
>>   Thanks,
>>   Alexandr.
>>
>> On 8/14/2015 10:33 AM, shilpi rastogi wrote:
>>> Hi All,
>>>
>>> Thanks Alexander for comments!
>>>
>>> I tried to incorporate all changes suggested by you.
>>>
>>> Please have a look
>>>
>>> http://cr.openjdk.java.net/~sgupta/7124238/webrev.05/
>>> http://cr.openjdk.java.net/~sgupta/7124238/webrev.04/
>>>
>>>
>>> Thanks
>>> Shilpi
>>>
>>> On 8/13/2015 7:37 PM, Alexander Scherbatiy wrote:
>>>> On 8/13/2015 2:51 PM, shilpi rastogi wrote:
>>>>> Hi All,
>>>>>
>>>>> Please review updated webrev
>>>>>
>>>>> http://cr.openjdk.java.net/~sgupta/7124238/webrev.03/
>>>>> http://cr.openjdk.java.net/~sgupta/7124238/webrev.02/
>>>>
>>>>   - I forgot to mention that the rule is to split line if it longer 
>>>> than 80 characters. Long lines  are annoying for side-by-side views.
>>>>    IDEs usually allow to configure right margin settings in a 
>>>> editor for this purposes.
>>>>   - It is better to define that createAndShowGUI() method throws 
>>>> Exception. In this case the try/catch block for L&F setting is 
>>>> unnecessary.
>>>>   - SwingUtilities.invokeAndWait() does not throw AWTException so 
>>>> its declaration can be removed.
>>>>
>>>>   Thanks,
>>>>   Alexandr.
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> Shilpi
>>>>>
>>>>>
>>>>> On 8/13/2015 12:56 PM, Alexander Scherbatiy wrote:
>>>>>> On 8/13/2015 8:16 AM, shilpi rastogi wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Please review a test bug fix
>>>>>>>
>>>>>>> TEST : 
>>>>>>> closed/javax/swing/plaf/basic/BasicHTML/4960629/bug4960629.java
>>>>>>> BUG ID - https://bugs.openjdk.java.net/browse/JDK-7124238
>>>>>>>
>>>>>>> Pleasemove  the test from closed repo to open repo.
>>>>>>>
>>>>>>> The webrev is: 
>>>>>>> http://cr.openjdk.java.net/~sgupta/7124238/webrev.01 add to open 
>>>>>>> repo
>>>>>>
>>>>>>     - The long lines should be split so they fit to a page
>>>>>>     - Exceptions should be re-thrown. In other case they are just 
>>>>>> printed but jtreg decides that a test is passed.
>>>>>>
>>>>>>   69                 try {
>>>>>>      70                     createAndShowGUI();
>>>>>>      71                 } catch (AWTException e) {
>>>>>>      72                     e.printStackTrace();
>>>>>>      73                 }
>>>>>>
>>>>>>    - The test methods can throw a general Exceptioninstead of 
>>>>>> several concrete ones. This usually makes a test more readable.
>>>>>>       It also allows to omit try/catch block for the the 
>>>>>> UIManager.setLookAndFeel() call.
>>>>>>
>>>>>>    Thanks,
>>>>>>    Alexandr.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~sgupta/7124238/webrev.00/ 
>>>>>>> <http://cr.openjdk.java.net/%7Ekshefov/8017187/webrev.diff/> - 
>>>>>>> diff with previous version of the  closed test.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Shilpi
>>>>>>
>>>>>
>>>>
>>>
>>
>