Hi Jaroslav, 1. I think it would be good to change the synopsis of the issue to match what the proposed change does.
It seems to me that something like: Add a new javax.management.annotation.ConstructorProperties annotation would be a better description. 2. I agree with Alan that logging might not be needed. I am not opposed to keeping it but then I'd suggest using one of the loggers defined in JmxProperties.java rather than creating a new logger object. 3. I was told recently that @since 1.9 should now be @since 9 (package-info + new annotation type) 4. ConstructorProperties.java: Are you going to annotate MemoryUsage? 5. I see you have updated existing tests to use the new annotation. I wonder whether it would have been better to clone them instead. On the other hand you have added a new test to test the legacy annotation, so maybe that was the right thing to do. Unless others object, keep it like that. Best regards, -- daniel On 08/10/15 13: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 Issue description: "MXBean currently supports model-specific types annotated with java.beans.ConstructorProperties that is tightly coupled with the client API. A MXBean developer will likely want to avoid using java.beans.ConstructorProperties if it ends up in the desktop module that their code doesn't want to pull in. In that case, the code has to write to achieve the same effort by defining the from(CompositeData) method." This patch adds a new annotation @javax.management.annotation.ConstructorProperties which can be used in stead of @java.beans.ConstructorProperties. This will allow the developers to use this convenience feature without introducing a bit strange dependency on java.desktop. For the backward compatibility purposes @java.beans.ConstructorProperties annotation will still be recognized by the JMX system but a) A warning will be logged about using a deprecated way to specify @ConstructorProperties b) If there is also @javax.management.annotation.ConstructorProperties annotation present on the same constructor then only this annotation will be considered. All the tests exercising the JMX related @ConstructorProperties functionality have been updated to use @javax.management.annotation.ConstructorProperties. Since this change is affecting public APIs the relevant CCC request has been filed and is in processing now. Thanks, -JB-
