----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1689/#review1705 -----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js <https://reviews.apache.org/r/1689/#comment3875> Small nit, I would prefer you call the second parameter listener instead of callback, since the name of your method has the word listener in it. http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js <https://reviews.apache.org/r/1689/#comment3874> Any particular reason why you chose to do the removeListener this way? I would expect the API to take in the listener you want to remove, not a token.... One disadvantage I could think of is you have to save off this token somewhere to use it later on if you want to remove it. Then again you also need to save off your listener if you want to remove it as well, so maybe there really is no difference. Also as far as consistency, are there other listener APIs in the container today? How do they do it? - Ryan On 2011-08-31 20:07:34, Dan Dumont wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1689/ > ----------------------------------------------------------- > > (Updated 2011-08-31 20:07:34) > > > Review request for shindig, Ryan Baxter and Stanton Sievers. > > > Summary > ------- > > Specification: > http://code.google.com/p/opensocial-resources/issues/detail?id=1210 > > Add 2 functions to the actions feature. > 1 to allow the container to register a listener for an action being run. The > listener would be run whenever an action matching the id provided was run. > 1 to allow the container to unregister(remove) any listener added with the > previous function. > > > This addresses bug SHINDIG-1612. > https://issues.apache.org/jira/browse/SHINDIG-1612 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js > 1163664 > > Diff: https://reviews.apache.org/r/1689/diff > > > Testing > ------- > > If anyone has any suggestions as to how to approach adding unit tests for > this, I'm all ears. > > It looks like that in order to get the actions api to hit this code, by > calling runAction, I'd need to somehow fake setting up a gadget site and > register a fake action. > > > Thanks, > > Dan > >
