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

[Alexander Scherbatiy]

   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
>>>>
>>>
>>
>