[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RE: RFR: 8189102: All tools should support -?, -h and --help
From:       "Lindenmaier, Goetz" <goetz.lindenmaier () sap ! com>
Date:       2017-11-30 9:18:54
Message-ID: f0767add3acc4cbc960ffa6603abe020 () sap ! com
[Download RAW message or body]

Thanks, Robert!

Best regards,
  Goetz.

> -----Original Message-----
> From: Robert Field [mailto:robert.field@oracle.com]
> Sent: Mittwoch, 29. November 2017 19:31
> To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>
> Cc: Kumar Srinivasan <kumar.x.srinivasan@oracle.com>; serviceability-dev
> (serviceability-dev@openjdk.java.net) <serviceability-
> dev@openjdk.java.net>; compiler-dev@openjdk.java.net; core-libs-
> dev@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.lindenmaier@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.srinivasan@oracle.com]
> >> Sent: Montag, 27. November 2017 17:43
> >> To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>
> >> Cc: core-libs-dev@openjdk.java.net; 'compiler-dev@openjdk.java.net'
> >> <compiler-dev@openjdk.java.net>; serviceability-dev (serviceability-
> >> dev@openjdk.java.net) <serviceability-dev@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.
> >>>
> >

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic