On 08/05/2017 09:45, Ujwal Vangapally wrote:
Thanks for the Review Daniel, Harsha
Please find the new webrev incorporating the review comments
webrev :
http://cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.01/
Looks good. In retrospect I wonder if renaming
getListenerIds/getListenerId into getClientListenerIds/getClientListenerId
was such a good idea given that 'listenerId' is an established
terminology in the specification [1] [2] (and 'listenerId' is used all
over the place in ClientNotifForwarder anyway).
Harsha, what do you think?
best regards,
-- daniel
[1] See @return in
http://download.java.net/java/jdk9/docs/api/javax/management/remote/rmi/RMIConnection.html#addNotificationListeners-javax.management.ObjectName:A-java.rmi.MarshalledObject:A-javax.security.auth.Subject:A-
[2] see @param listenerIds in
http://download.java.net/java/jdk9/docs/api/javax/management/remote/rmi/RMIConnection.html#removeNotificationListeners-javax.management.ObjectName-java.lang.Integer:A-javax.security.auth.Subject-
Thanks,
Ujwal
On 5/8/2017 12:03 PM, Daniel Fuchs wrote:
Hi Ujwal,
For consistency I think the new getListenerIds method should:
a) either return an array of Integer, even if it contains only 1
Integer:
1. The name of the method implies that an array is returned
2. You will need the array when you call
connection.removeNotificationListeners anyway.
or b) the other possibility is to remove the 's' at the end of the
method.
Both would be acceptable.
best regards,
-- daniel
On 04/05/17 07:59, Ujwal Vangapally wrote:
corrected webrev link :
http://cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.00/
On 5/4/2017 12:14 PM, Ujwal Vangapally wrote:
Kindly review the changes made for below bug
Problem description and solution are explained in comments section
https://bugs.openjdk.java.net/browse/JDK-6515161
diff for*ClientNotifForwarder.java *might be a bit confusing as it
shows the method name
removeNotificationListener is modified to getListenerIds and new
method removeNotificationListener is introduced.
Actual change is new method getListenerIds is introduced and it is
called in removeNotificationListener method without
affecting the functionality of removeNotificationListener.
webrev :
cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.00/
Thanks,
Ujwal,
