Hi Roger,
Thank you for detailed reviewing, fixed as you suggested except below
comment:
81-83/93/95: use SHARE.resolve(xxx).toString to compute the paths.
As the code is constructing a class path with 2 paths rather than
constructing a path.
new webrev: http://cr.openjdk.java.net/~mli/8166142/webrev.03/
Thank you
-Hamlin
On 2017/6/1 0:47, Roger Riggs wrote:
Hi Hamlin,
RenamePackageTest.java:
- 48,61: rename "sutup" -> "setup"
81-83/93/95: use SHARE.resolve(xxx).toString to compute the paths.
ClassPathTest.java:
42: add a space before "{"
56: Generally, exception messages should not end in "."; they are phrases
so they could be printed with context.
In this specific case it is not important, just a pattern to follow
consistently.
Thanks, Roger
On 5/26/2017 9:14 PM, Hamlin Li wrote:
Hi Roger,
Thank you for catching it, new webrev:
http://cr.openjdk.java.net/~mli/8166142/webrev.02/
Thank you
-Hamlin
On 2017/5/27 3:30, Roger Riggs wrote:
Hi Hamlin,
SubclassTest.java:37: typo" excepiton" -> exception
SubclassDataLossTest.java:
103/104: Adding the class loader close() calls isn't very effective
since if there is an exception they are not
closed and the creation in a static initializer is mismatched with
main() code.
It would be cleaner to open in main() using try-with-resources and
close them in a finally clause.
And pass the loaders to the test function.
But for simplicity, perhaps just leave out the new calls to
ldr1/2.close.
(There is something odd about the webrev links, the nagivation links
don't work as expected. but that's transient)
Thanks, Roger
On 5/26/2017 12:26 AM, Hamlin Li wrote:
Hi Roger,
Thank you for detailed review. Fixed as you suggested, new webrev:
http://cr.openjdk.java.net/~mli/8166142/webrev.01/
Thank you
-Hamlin
On 2017/5/26 2:54, Roger Riggs wrote:
Hi Hamlin,
For imports they should import specific classes, wildcards are not
used.
maskSyntheticModifier/Test.java
consTest/Test.java
deserializeButton/Test.java
test/java/io/Serializable/packageAccess/Driver.java: the name
"Driver" is not descriptive and should be.
Each Driver.java should have a different and
functional/descriptive name.
Better yet, there should be a single program that creates jar
files using command line arguments
that can be invoked depending on the test.
There is a mix of Driver's copying files vs the test program
copying; can that be made more uniform.
Test.java files should have descriptive/functional names. (Even if
the duplicating the directory)
Regards, Roger
On 5/24/2017 5:09 AM, Hamlin Li wrote:
Would you please review the below patch?
bug: https://bugs.openjdk.java.net/browse/JDK-8166142
webrev: http://cr.openjdk.java.net/~mli/8166142/webrev.00/
Thank you
-Hamlin
