+1
Thanks Ujwal!
-- daniel
On 09/05/17 06:38, Ujwal Vangapally wrote:
Hi Harsha, Daniel,
updated method names as getListenerIds/getListenerId
webrev :
http://cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.02/
On 5/8/2017 9:57 PM, Daniel Fuchs wrote:
Hi Harsha,
Well, I think both methods should have the same name
and I don't see any strong reason for changing the old
name - since there is no such thing as a 'ClientListener'
(there are only NotificationListeners).
best regards,
-- daniel
On 08/05/2017 17:13, Harsha Wardhana B wrote:
Hi Daniel,
The term 'listenerid' is used in conjunction with method name/object
field which adds context about the term 'listenerid'. Having a
standalone method name as getClientListenerId is less ambiguous compared
to method name : getListenerId.
I don't really have a strong opinion on this and am okay with either
names.
-Harsha
On Monday 08 May 2017 03:16 PM, Daniel Fuchs wrote:
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,
