Hi Stuart, Thanks for picking up the review.
> - java/util/jar/JarFile.java > - java/util/jar/Manifest.java [patch4] > - java/util/logging/LogManager.java [patch2] > - java/util/prefs/Preferences.java > - java/util/prefs/XmlSupport.java > - java/util/zip/ZipEntry.java [patch2] > > Did I get everything? I think you've addressed all the review comments that > have come in so far. (Other reviewers, please recheck the webrev!) Yep, that looks like all of them. > In addition, I have the following comments: > > JarFile.java -- > > 242 ZipEntry ze = (ZipEntry)enum_.nextElement(); > 621 ZipEntry ze = (ZipEntry) enum_.nextElement(); > 676 ZipEntry e = (ZipEntry) entries.nextElement(); > > I think these are all unnecessary casts now, since at 242 and 621 the return > type from nextElement is <? extends ZipEntry> and at 676 it's JarEntry, and > JarEntry is a subclass of ZipEntry. So, these casts can probably be removed > even though they don't generate warnings. An alternative would be to change > the type of e at 676 to JarEntry; not sure if this would be better. > > LogManager.java -- > > 203 Logger.getGlobal().setLogManager(manager); > 204 manager.addLogger(Logger.getGlobal()); > > The doc recommends replacing mentions of the global field with a call to > getGlobal(), and this was done here, however sometimes what's in the doc > doesn't necessarily apply to library code. Can someone who's more familiar > with logging verify this change is correct? > > 418 Class<?> clz = ClassLoader.getSystemClassLoader().loadClass(word); > 419 Handler hdl = (Handler) clz.newInstance(); > > Minor nit: extra space was before 'clz' in the original code, I think, to > make it line up with 'hdl'. Maybe spacing needs to be rearranged to keep > them lined up. (I see this a couple other places in the file, but it's not > too prevalent.) The alternative is to have a single space between the type > and the variable name. These all look sensible, I'll implement them. > Mike, if you end up needing to update this patch further, it might be > easiest if you just sent all the changes in a single patch file, i.e. the > output of 'hg diff'. I can then apply it to the tip and generate a webrev > quite easily. No problem. I'll drop a new patch in tomorrow. Mike.
