Thank you, Mandy. I just fixed it follow your comments and get it pushed. Regards, Chris
> On 29 Mar 2018, at 12:48 PM, mandy chung <mandy.ch...@oracle.com > <mailto:mandy.ch...@oracle.com>> wrote: > > > > On 3/29/18 9:59 AM, Chris Yin wrote: >> Please review the change to merge 2 package access tests and move to >> OpenJDK, thanks >> >> bug: https://bugs.openjdk.java.net/browse/JDK-8196668 >> <https://bugs.openjdk.java.net/browse/JDK-8196668> >> <https://bugs.openjdk.java.net/browse/JDK-8196668> >> <https://bugs.openjdk.java.net/browse/JDK-8196668> >> webrev: http://cr.openjdk.java.net/~xyin/8196668/webrev.00/ >> <http://cr.openjdk.java.net/~xyin/8196668/webrev.00/> >> <http://cr.openjdk.java.net/~xyin/8196668/webrev.00/> >> <http://cr.openjdk.java.net/~xyin/8196668/webrev.00/> >> > > Looks okay. Minor comments: > 88 throw new RuntimeException("Unexpected > AccessControlException", > 89 ace); > > 92 throw new RuntimeException("Test failed with unexpected > exception", > 93 ex); > > Nit: each throw statement can be merged in 1 line. > > test/jdk/java/lang/SecurityManager/empty.policy > can you add a comment saying this is an empty policy. > > No need to generate a new webrev. You can fix it before you push. > > Mandy
