> On 2011-08-24 00:22:17, Ryan Baxter wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/selection/selection_test.js, > > line 84 > > <https://reviews.apache.org/r/1615/diff/1/?file=34111#file34111line84> > > > > Shouldn't there be a test for removeListener as well? > > Matthew Hatem wrote: > Yes I will add one. > > Matthew Hatem wrote: > I take that back. It's currently not possible to test this without > exposing things that should be private. > > Is there plan to upgrade/improve the unit testing in Shinding?
Ah ok. Not that I know of, but we are open to suggestions if you have ideas :) > On 2011-08-24 00:22:17, Ryan Baxter wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/selection/selection_container.js, > > line 96 > > <https://reviews.apache.org/r/1615/diff/1/?file=34109#file34109line96> > > > > How does this ever get called? Shouldn't this be called when the > > gadget calles removeListener? If so it should be called from your router > > function...right? > > Matthew Hatem wrote: > This allows the container to add and remove listeners, has nothing to do > with gadget level API. > > Ryan Baxter wrote: > So when the gadget adds a listener we add it to a list in the container, > but when a gadget removes a listener we never remove it from the list in the > container....why? > > Matthew Hatem wrote: > We've talked about this before. Each gadget adds only one listener > (lazily) to the list in the container which proxies events to (potentially > many) listeners registered with a local list at the gadget. This cuts down > on the number of RPCs that are necessary to notify everyone. > > You might argue that I need a lifecycle listener to cleanup the proxy > listeners. I don't want to hold up this patch for that though, I'll open a > separate issue for that. Ah yes now I remember, thanks for the refresher. Yes it would be nice to clean this up. - Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1615/#review1604 ----------------------------------------------------------- On 2011-08-23 23:06:15, Matthew Hatem wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1615/ > ----------------------------------------------------------- > > (Updated 2011-08-23 23:06:15) > > > Review request for shindig and Ryan Baxter. > > > Summary > ------- > > https://issues.apache.org/jira/browse/SHINDIG-1591 > > > This addresses bug SHINDIG-1591. > https://issues.apache.org/jira/browse/SHINDIG-1591 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/conservcontainer/sample-actions-runner.xml > 1160436 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/conservcontainer/sample-selection-listener.xml > 1160436 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/selection/feature.xml > 1160436 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/selection/selection.js > 1160436 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/selection/selection_container.js > 1160436 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/selection/taming.js > 1160436 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/selection/selection_test.js > 1160436 > > Diff: https://reviews.apache.org/r/1615/diff > > > Testing > ------- > > Passes all unit tests. > > > Thanks, > > Matthew > >