<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