Hi Chris,
pushed, thank you very much!
-Felix
On 2017/5/30 17:44, Chris Hegarty wrote:
On 29/05/17 16:17, Felix Yang wrote:
Hi Chris,
please review the updated webrev below. Comments inline.
http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.03/
Thanks Felix.
..
2) test/java/net/URLClassLoader/getresourceasstream/policy
Please add a copyright/header.
Checked existing test policy files, all of them have no
copyright/header. So I hesitated to add here. Please confirm.
The code is not consistent, but please do add the copyright
header here ( we'll fix the other files at a later time ).
Added.
...
4) Why has the test for 5077773 simply been removed?
As priorly commented, the test is probably not necessary.
Because it actually quits immediately after prerequisite checking for
"javax/swing/text/rtf/charsets/mac.txt". According with JDK-5077773, it
is to cover endorsed scenarios, while endorsed mechanism has been
removed in JDK 9.
Ok, sorry for not seeing the previous reply.
I am finished reviewing this. No need for an updated webrev, for
the license header.
-Chris.
-Felix
-Chris.
On 27/05/17 02:15, Felix Yang wrote:
Hi Roger,
thanks for the comments. Please see the new webrev
http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.02/
-Felix
On 2017/5/27 3:50, Roger Riggs wrote:
Hi Felix,
fyi, there is a new version of webrev.ksh that provides convenient
next and previous links.
http://hg.openjdk.java.net/code-tools/webrev/raw-file/d26c194772db/webrev.ksh
CloseTest.java: 66; checking WORK_DIR *after* calling setup() does
not
make sense.
Ditto: closetest/GetResourceAsStream.java
Removed such check, since the test depends on several system
properties("test.src", "user.dir"). It is not expected to run outside
jtreg.
-Felix
URLConnection/6212146/Test.java:47 typo: ULR -> URL
Regards, Roger
On 5/25/2017 11:14 PM, Felix Yang wrote:
Hi Roger,
please review the updated webrev:
http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/
Thanks,
Felix
On 2017/5/26 3:24, Roger Riggs wrote:
Hi Felix,
closetest/CloseTest:
32: Please put the @modules directives together (and not repeat)
Fixed
-Felix
44: do not use wildcard imports (reconfigure your IDE if necessary
to avoid accidents).
Fixed and also organized imports in other test files
-Felix
63: setup() is invoked twice... remove 1 or explain why 2 calls
Fixed
-Felix
69, 78,90,91,... : space in method call is not proper style
Fixed and also formatted other history codes. Just format only
without logic change.
Not restricted to this test. I only formatted history codes
"nearby",
to avoid hiding real logic changes in the patch.
-Felix
sealing/CheckSealed.java:
- Why is @test block removed?
Should be converted to @run main CheckSealed
There was a checksealed.sh. I replaced it with CheckSealedTest.java
and declared @test there.
http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/test/java/net/URLClassLoader/sealing/CheckSealedTest.java.html
BTW, you may noticed test/java/net/URLClassLoader/B5077773.* were
removed. That is because corresponding prerequisite was removed even
in JDK 8.
Perhaps then add a comment to CheckSealed.java indicating it is
compiled and executed by CheckSealedTest.java.
Roger
-Felix
Regards, Roger
On 5/25/2017 4:08 AM, Felix Yang wrote:
Hi there,
please review following patch to convert all shell cases under
java/net to plain java codes.
Webrev:
http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.00/
Bug:
https://bugs.openjdk.java.net/browse/JDK-8166139
Thanks,
Felix
