Reviewed Nashorn changes. All fine. -Sundar
On 11/28/2016 8:17 PM, Chris Hegarty wrote: > On 24 Nov 2016, at 15:25, Alan Bateman <alan.bate...@oracle.com> wrote: >> ... >> To get going, I've put the webrevs with a snapshot of the changes in jake >> here: >> http://cr.openjdk.java.net/~alanb/8169069/0/ > Overall this look very good. I ran through most of the changes in the jdk > repo, > just a few small comments. > > 1) SuppressWarnings.java typo element - > elementS > > 38 * However, note that if a warning is suppressed in a {@code > 39 * module-info} file, the suppression applies to elementS within the > 40 * file and <em>not</em> to types contained within the module. > > 2) jartool Main.java > Maybe concealedPackages should have a comment about its use ( it is > used in the Validator, and not by Main at all ). > > 3) MethodHandles.java privateLookupIn > It might be clearer if the third bullet used {@code lookup}, or 'caller > lookup’, or ‘given lookup'? > > The CALLER lookup has the {@link Lookup#MODULE MODULE} lookup mode. > > 4) ServiceLoader > {@code ExtendedCodecsFactory} > 111 * will be treated as a provider factory and {@code > 112 * ExtendedCodecsFactory.provider()} will be invoked to INSTANTIATE the > 113 * provider. > > Is 'instantiate' strictly true here? Should it simply be ‘return' > > 206 * <li> If a named module declares more than one provider then the > providers > 207 * are located in the order that they appear in the {@code > provides} table of > 208 * the {@code Module} class file attribute ({@code > module-info.class}). </li> > > Wow. I assume the JLS, or otherwise, will specify that the order in which > the > providers are listed in the module-info be preserved, no? Maybe this item > could > mention that. The class file reference can still be kept, but seems a bit > low-level > for developers. > > -Chris.
