<Swing Dev> REM: Re: [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/31/2015 2:07 PM, Alexander Zvegintsev wrote:
> Hi Shilpi,
> the fix looks good to me
>
> Thanks,
>
> Alexander.
>
> On 08/31/2015 12:00 PM, shilpi rastogi wrote:
>> Hi All,
>>
>> Please review updated webrev
>>
>> open: 
>> http://cr.openjdk.java.net/~psadhukhan/shilpi/webrev-7124238/open-7124238/webrev/ 
>>
>> closed: 
>> http://cr.openjdk.java.net/~psadhukhan/shilpi/webrev-7124238/closed-7124238/webrev/ 
>>
>>
>>
>> Thanks
>> Shilpi
>>
>> On 8/24/2015 7:44 PM, Alexander Zvegintsev wrote:
>>> 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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>