On 8.10.2015 14:15, Alan Bateman wrote:
On 08/10/2015 12:49, Jaroslav Bachorik wrote:
Please, review the following change
Issue : https://bugs.openjdk.java.net/browse/JDK-7199353
Webrev: http://cr.openjdk.java.net/~jbachorik/7199353/webrev.00/top
http://cr.openjdk.java.net/~jbachorik/7199353/webrev.00/jdk
I see this patch is against jigsaw/jake but I assume this will go into
JDK 9 via jdk9/dev. No need for it to be pushed via the jake forest, right?
The patch is adding a new exported package to java.management - so it
would have to be adjusted to the way jdk9 defines modules right now (eg.
modules.xml). And then do this again for jigsaw/jake
I would, personally, prefer to do the change only once (in jigsaw/jake)
and then just let the change trickle back to jdk9/dev once jigsaw is merged.
That said, there is some wording in MXBean that will need adjustments
for modules. Specifically the wording around subset Profiles of Java SE
will need to be modernized a bit to cover any Java SE run-time that
doesn't have the java.desktop module (or java.beans package).
Should it be done in one change? I mean, it is not really related to @CP
changes. Probably a separate RFE for the docs update?
The approach and the new @CS trumping the old looks good. I'm not sure
that I agree with logging a warning when
java.beans.ConstructorProperties is used. I would be tempted to leave
that out.
The idea is that we want users to stop using @j.b.CP for JMX purposes.
So we might as well warn them about the suboptimal solution they are
using. But I don't insist on the logging.
The updates to the javadoc mostly okay. In the paragraph on "For
backward compatibility .." then it probably should make clear that this
annotations works in exactly the same way as the
javax.management.annotation.CS.
Ok
The webrev makes it looks like the beans @CS has been renamed, is this a
hg copy?
Yep.
I see this patch adds a package.html, any reason not to use
package-info.java here?
Copy/paste :(
I assume LegacyConstructorPropertiesTest's header will be fixed up so
that the copyright header is at the top.
Yep, this one slipped through ...
Thanks,
-JB-
-Alan
