RFR: JDK-7094818, JDK-8132861 and JDK-8134733
Hello, http://cr.openjdk.java.net/~peytoia/7094818.8132861.8134733/webrev.00/ Please review the fix for the following bugs: https://bugs.openjdk.java.net/browse/JDK-7094818 https://bugs.openjdk.java.net/browse/JDK-8132861 https://bugs.openjdk.java.net/browse/JDK-8134733 Thanks, -- Yuka
Re: Review Request:JDK-8163350-LocaleProviderAdapter Preference list retrieved is wrong, when -Djava.locale.providers=COMPAT
Hi Masayoshi, Thanks for the review. please have a look at updated webrev http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.04/ Thanks, Rachna On 8/16/16 1:16 PM, Masayoshi Okutsu wrote: Hi Rachna, The fix looks good to me. But the test should be changed. - It's unnecessary to statically import sun.util.locale.provider.LocaleProviderAdapter.Type. - Variable name PreferenceList should be preferenceList. - No need to initialize preferenceList with an ArrayList. Thanks, Masayoshi On 8/10/2016 2:31 AM, Rachna Goel wrote: Hi Naoto, Thanks for the review. Please have a look at updated patch: http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.03/ Thanks, Rachna On 8/9/16 9:39 PM, Naoto Sato wrote: Hi Rachna, I should've caught it at the internal review, but here are some cosmetic comments to the test case. - I'd prefer a blank line between import statements and the main method. - The argument to main() should be (String[] args) - Indentation is incorrect at line 39 - Needs parens for the if statement at line 41. Naoto On 8/9/16 3:51 AM, Rachna Goel wrote: Hi, Please review fix for JDK-8163350. Bug: https://bugs.openjdk.java.net/browse/JDK-8163350 Webrev: http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.02/ Fix: If user specifies either CLDR or COMPAT via System Property, FALLBACK should not be implicitly included. It is meant to be included when no ResourceBundleBased Adapters are available. Thanks, Rachna
RFR: JDK-8163362-Reconsider reflection usage in java.awt.font.JavaAWTFontAccessImpl class
Hi, Please review fix for JDK-8163362. https://bugs.openjdk.java.net/browse/JDK-8163362 Webrev: http://cr.openjdk.java.net/~rgoel/JDK-8163362/webrev.01/ This is a cleanup fix in which Reflection usage in JavaAWTFontAccessImpl class was removed. Thanks, Rachna
RFR: JDK-8135055.java.util.Date.after(java.sql.Timestamp ) does not return correct results
Hi, Please review fix for JDK-8135055. Bug: https://bugs.openjdk.java.net/browse/JDK-8135055 Webrev: http://cr.openjdk.java.net/~rgoel/JDK-8135055/webrev.02/ Fix is to return getTime() if argument to getMillisOf() is an instance of java.sql.TimeStamp. Thanks, Rachna
Re: RFR: JDK-7094818, JDK-8132861 and JDK-8134733
Looks good to me. Masayoshi On 8/22/2016 10:31 AM, Yuka Kamiya wrote: Hello, http://cr.openjdk.java.net/~peytoia/7094818.8132861.8134733/webrev.00/ Please review the fix for the following bugs: https://bugs.openjdk.java.net/browse/JDK-7094818 https://bugs.openjdk.java.net/browse/JDK-8132861 https://bugs.openjdk.java.net/browse/JDK-8134733 Thanks, -- Yuka
Re: Review Request:JDK-8163350-LocaleProviderAdapter Preference list retrieved is wrong, when -Djava.locale.providers=COMPAT
Looks good to me. Masayoshi On 8/22/2016 1:10 PM, Rachna Goel wrote: Hi Masayoshi, Thanks for the review. please have a look at updated webrev http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.04/ Thanks, Rachna On 8/16/16 1:16 PM, Masayoshi Okutsu wrote: Hi Rachna, The fix looks good to me. But the test should be changed. - It's unnecessary to statically import sun.util.locale.provider.LocaleProviderAdapter.Type. - Variable name PreferenceList should be preferenceList. - No need to initialize preferenceList with an ArrayList. Thanks, Masayoshi On 8/10/2016 2:31 AM, Rachna Goel wrote: Hi Naoto, Thanks for the review. Please have a look at updated patch: http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.03/ Thanks, Rachna On 8/9/16 9:39 PM, Naoto Sato wrote: Hi Rachna, I should've caught it at the internal review, but here are some cosmetic comments to the test case. - I'd prefer a blank line between import statements and the main method. - The argument to main() should be (String[] args) - Indentation is incorrect at line 39 - Needs parens for the if statement at line 41. Naoto On 8/9/16 3:51 AM, Rachna Goel wrote: Hi, Please review fix for JDK-8163350. Bug: https://bugs.openjdk.java.net/browse/JDK-8163350 Webrev: http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.02/ Fix: If user specifies either CLDR or COMPAT via System Property, FALLBACK should not be implicitly included. It is meant to be included when no ResourceBundleBased Adapters are available. Thanks, Rachna
Re: RFR: JDK-8135055.java.util.Date.after(java.sql.Timestamp ) does not return correct results
Looks good to me. Masayoshi On 8/22/2016 1:51 PM, Rachna Goel wrote: Hi, Please review fix for JDK-8135055. Bug: https://bugs.openjdk.java.net/browse/JDK-8135055 Webrev: http://cr.openjdk.java.net/~rgoel/JDK-8135055/webrev.02/ Fix is to return getTime() if argument to getMillisOf() is an instance of java.sql.TimeStamp. Thanks, Rachna
Re: RFR: JDK-8163362-Reconsider reflection usage in java.awt.font.JavaAWTFontAccessImpl class
Looks good to me. Masayoshi On 8/22/2016 1:26 PM, Rachna Goel wrote: Hi, Please review fix for JDK-8163362. https://bugs.openjdk.java.net/browse/JDK-8163362 Webrev: http://cr.openjdk.java.net/~rgoel/JDK-8163362/webrev.01/ This is a cleanup fix in which Reflection usage in JavaAWTFontAccessImpl class was removed. Thanks, Rachna
