Request for Review: Java SE 8 Compact Profiles

[Mandy Chung]

Hi David,

On 12/20/2012 10:18 PM, David Holmes wrote:
> jdk repo: http://cr.openjdk.java.net/~dholmes/8004265/webrev.jdk/

I reviewed the src/test changes in the jdk repo and have a few comments:

Attributes.java:
  568    * {@code Name} object for {@code Profile} manifest attribute used
  569    * by applications packaged as JAR files to indicate the minimum
  570    * profile required to execute the application.

The Profile attribute can be specified in both applications and libraries.
Shoud L569 be changed with  s/applications/applications or libraries?

Pack200.java
   I think the default implementation for addPropertyChangeListener
   and removePropertyChangeListener requiring a non-null listener is
   a right choice.  On the other hand, I found that the current pack200
   implementation allows the given listener be null that seems to be
   a bug and the Pack200 class spec also specifies to throw NPE if null
   argument is passed to a method and looks like what you have

sun.misc.URLClassPath
   L808: typo "attribtue"

   L820: An empty "Profile" attribute is invalidand Version.supportsProfile
   returns false if requiredProfile parameter is empty even if the runtime
   is a full JRE. This is fine but I was wondering if the exception message
   can indicate if the attribute value is invalid to help diagnosis.

   L1000: looks like the convention is to use brackets even there is a
   single statement in the if-statement body.

sun.tools.jar.Main
   It would be helpful if the jar tool checks if the input profile
   name to the -p option is valid and outputs error.

UnsupportedProfileException
   L29: probably better to link to the javadoc
        {@link Attributes.Name#Profile Profile}
   L38 and 44: {@code UnsupportedProfileException}

make/tools/src/build/tools/RemoveMethods.java
    L100: maybe print the method signature rather than just 'name'
    L106: typo "no longed" -> "no longer"

The tests are hardcoded with the profile name and uses 
Version.profileName().
Will there be a system property for the profile name?

Otherwise, looks okay.

Mandy