>> Please review the following changeset: >> >> webrev: http://cr.openjdk.java.net/~sdrach/8150680/webrev.00/index.html >> <http://cr.openjdk.java.net/~sdrach/8150680/webrev.00/index.html> >> issue: https://bugs.openjdk.java.net/browse/JDK-8150680 >> <https://bugs.openjdk.java.net/browse/JDK-8150680> >> >> The issue calls for reconsidering the JarFile.Release enum. A comment in >> the bug report suggests replacing JarFile.Release with Runtime.Version, and >> that’s what I did. Specifically I removed the enum, changed the constructor >> to accept a Runtime.Version object instead of a JarFile.Release object, >> updated all places in the JDK that invoked the constructor and updated all >> tests. >> > Moving to Runtime.Version seems right but doesn't the javadoc for the > constructor need to be updated to make it clear how it behavior when invoking > with something like Version.parse("7.1") ? If I read the code correctly then > this will be accepted and getVersion() will return 7.1.
Yes, it needs to be updated and it needs to be fixed. Thanks for finding that. > > Fields or methods is another discussion point for the base and runtime > versions. My thinking is, in this case fields and methods are equivalent, the method not giving any more flexibility than a field. For example the method JarFile.baseVersion will just return the value contained in the private final static field BASE_VERSION. Or the public final static field BASE_VERSION can be directly accessed. I see no advantage of a method here. But I’m willing to be enlightened. > > -Alan.
