Hi,
Besides my private comments to you, there are 2-3 internal tests
which fail.
Have you run all the langtools tests ? There are 4 Windows tests
that fail with langtools:
jdk/javadoc/doclet/testHelpOption/TestHelpOption.java
jdk/javadoc/tool/CheckResourceKeys.java
jdk/javadoc/tool/ToolProviderTest.java
tools/javap/InvalidOptions.java
tools/jdeps/MultiReleaseJar.java
This changeset needs to be vetted thoroughly using internal build and
test systems.
Any Oracle engineer willing to sponsor this ?
Kumar
On 11/28/2017 3:16 AM, Lindenmaier, Goetz wrote:
Hi,
I uploaded a new webrev:
http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.04/
This includes the changes
- to jshell requested by Robert
- to the test as requested by Kumar.
See also incremental patch and the test output including all the
help messages referenced in the webrev.
Best regards,
Goetz.
-----Original Message-----
From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com]
Sent: Montag, 27. November 2017 17:43
To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>
Cc: core-libs-dev@openjdk.java.net; 'compiler-...@openjdk.java.net'
<compiler-...@openjdk.java.net>; serviceability-dev (serviceability-
d...@openjdk.java.net) <serviceability-...@openjdk.java.net>
Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
I looked at some of the components I maintain, and they look good.
Wrt. the test:
1. The local variables/fields don't comply with the coding conventions,
we have been
following in the JDK.
2. Hmm, someone parked a .ini file in bin directory, later removed,
perhaps on windows simply check for .exe ? Should a check exist
to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool.
172 static boolean notATool(String file) {
173 file = file.toLowerCase();
174 if (file.endsWith(".dll") ||
175 file.endsWith(".map") ||
176 file.endsWith(".pdb") ||
177 file.equals("server")) { // server subdir on windows.
178 return true;
179 }
180 return false;
181 }
3. Typo in comment
201 // Check whether 'flag' appears in 'line' as a word of itself. I must
not
s/I must/It must/
4. There is a method to check for isEnglishLocale in TestHelper, perhaps
use it
to have the test bale out in non english locales as early as possible ?
5. Has this been tested on all platforms ? do you need help testing it ?
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this:
http://cr.openjdk.java.net/~goetz/wr17/8189102-
helpMessage/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8189102
CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to
give a more complete overview of currently supported flags
for the CSR:
http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-
October/028615.html
Best regards,
Goetz.
