On 14.10.2015 15:24, Alan Bateman wrote:
On 14/10/2015 10:34, Jaroslav Bachorik wrote:
Round 2 webrev: http://cr.openjdk.java.net/~jbachorik/7199353/webrev.01
Changes against round 1:
* @javax.management.ConstructorProperties (was
@javax.management.annotation.ConstructorProperties)
* diff is against the current jdk9 (eg. not the jigsaw repo)
* removed warning when @j.b.ConstructorProperties is used
* changed the wording in the reconstruction rules in @MXBean as
recommended
* changed the issue synopsis as recommended
JDK JMX tests are still passing completely.
Good to see the new @CP in the right place.
"When @java.beans.ConstructorProperties is used then rule 2 is not
applicable to subset Profiles of Java SE that do not include the
java.beans package."
I think this needs to say that when @java.beans.ConstructorProperties is
present, and @javax.management.ConstructorProperties is not present,
then rule 2 is not applicable.
Eg. "When only @java.beans.ConstructorProperties is used then rule 2 is
not applicable to subset Profiles of Java SE that do not include the
java.beans package." ?
I realize ConstructorProperties.java has been copied from the beans
ConstructorProperties.java but I think a bit of clean-up is in order.
L37 is mis-aligned, the example could use {@code ...}, the comment on
the value method could be cleaned up.
Also in the javadoc example then shouldn't the getXXX methods be public?
One other thing that comes to mind is whether we need any updates to
javadoc in the java.beans area. I could imagine someone in the IDE
typing ConstructorProperties and accidentally selecting
@javax.management.ConstructorProperties and getting confused. I'm mostly
wondering if the there should be someone in the java.beans javadoc to
make it clear that @javax.management.ConstructorProperties is ignored.
Hm, shouldn't we name the new annotation differently then?
@ConstructorMapping ? It is not mandatory that we keep the actual name -
we are changing the package anyway ...
-JB-
-Alan.
