Thanks, Robert! Best regards, Goetz.
> -----Original Message----- > From: Robert Field [mailto:robert.fi...@oracle.com] > Sent: Mittwoch, 29. November 2017 19:31 > To: Lindenmaier, Goetz <goetz.lindenma...@sap.com> > Cc: Kumar Srinivasan <kumar.x.sriniva...@oracle.com>; serviceability-dev > (serviceability-...@openjdk.java.net) <serviceability- > d...@openjdk.java.net>; compiler-...@openjdk.java.net; core-libs- > d...@openjdk.java.net > Subject: Re: RFR: 8189102: All tools should support -?, -h and --help > > Thumbs up for JShell changes. > > -Robert > > > On Nov 28, 2017, at 3:16 AM, Lindenmaier, Goetz > <goetz.lindenma...@sap.com> 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. > >>> > >
