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

shilpi rastogi shilpi.rastogi at oracle.com
Wed Sep 2 05:35:22 UTC 2015


Hi All,

I got two approvals for this fix. Could somebody please push it for me.

Thanks,
Shilpi
On 9/1/2015 7:18 PM, Alexander Scherbatiy wrote:
>
>  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
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>




More information about the swing-dev mailing list