[Asterisk-code-review] ARI: Add the ability to subscribe to all events (asterisk[13])

[Ashley Sanders]

Ashley Sanders has posted comments on this change.

Change subject: ARI: Add the ability to subscribe to all events
......................................................................


Patch Set 2:

(5 comments)

https://gerrit.asterisk.org/#/c/1193/2/res/ari/resource_events.c
File res/ari/resource_events.c:

Line 195: 
I am sure I am missing something obvious here... 

I wonder why you chose to create two new handlers to replace this one, rather than modifying the signature of the existing function to accept an extra argument? The data you needed to inform the handler if this was to be a catch-all subscription or a targeted one, is in the arguments supplied to this function and the established cb function (args->subscribe_all).


https://gerrit.asterisk.org/#/c/1193/2/res/stasis/app.c
File res/stasis/app.c:

Line 1180: return unsubscribe(app, "channel", S_OR(channel_id, CHANNEL_ALL), 0);
Since unsubscribe can determine from kind and a NULL channel id if the id should be CHANNEL_ALL, you could keep the former line of: 

return unsubscribe(app, "channel", channel_id, 0);


Line 1255: return app_unsubscribe_bridge_id(app, bridge ? bridge->uniqueid : BRIDGE_ALL);
Since  app_unsubscribe_bridge_id only uses the bridge id for invoking unsubscribe, and since unsubscribe can determine from kind and a NULL bridge id if the id should be BRIDGE_ALL, you could keep the former line of: 

return app_unsubscribe_bridge_id(app, bridge->uniqueid);


Line 1264: return unsubscribe(app, "bridge", S_OR(bridge_id, BRIDGE_ALL), 0);
Since unsubscribe can determine from kind and a NULL bridge id if the id should be BRIDGE_ALL, you could keep the former line of: 

return unsubscribe(app, "bridge", bridge_id, 0);


Line 1356: return unsubscribe(app, "endpoint", S_OR(endpoint_id, ENDPOINT_ALL), 0);
Since unsubscribe can determine from kind and a NULL endpoint id if the id should be ENDPOINT_ALL, you could keep the former line of: 

return unsubscribe(app, "endpoint", endpoint_id, 0);


-- 
To view, visit https://gerrit.asterisk.org/1193
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a943b4db24442cf28bc64b24bfd541249790ad6
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes