Hi Christoph, Sorry for the late reply but I was out of the office Friday > On Nov 15, 2019, at 3:40 AM, Langer, Christoph <christoph.lan...@sap.com> > wrote: > > Hi Lance, > > thanks for reviewing. I've addressed your points: > >> I would suggesting moving the code added to the constructor for checking >> the releaseVersion/multi-release properties to a method and grouping it >> with the other methods added for supporting MR jars around line 1396. >> (keeps it easier to follow and maintain) > > Done that.
Thank you. I do think however we need to change the name of the method to perhaps “checkReleaseVersionProperty” as the current name “initializeMultiReleaseJar” does not seem quite right to me. I would also update the comment for the method to something like: ---------------- Checks to see if the Zip File System property “releaseVersion” has been specified. If it has not been specified then check for the existence of the “multi-release” property. If set and the file represents a multi-release JAR, determine the runtime version to use. ---------------- > >> Could you also add a comment above the isMultiReleaseJar method to for >> clarity going forward (I know there was not one there before) > > Done, too. Thank you. I think we can tweak the comment slightly to something like ——————— Returns true if the Manifest main attribute “Multi-Release” is set to true; false otherwise -------------------- > >> I might change the name of the lookup field in the method makeParentDirs >> with the addition of the Function lookup on line 126. > > I chose to change the name of the global field in line 126 to be named > mrlookup. Also updated the comment. I am not sure “mrlookup” is the best name as this field is used with or without a multi-release jar. Perhaps “IndexNodeNameLookup” or “entryLookup" > > This is the new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234089.2/ > > Cheers > Christoph Thank you again! Best Lance > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
