> On Sept. 6, 2012, 6:43 p.m., Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js,
> >  line 136
> > <https://reviews.apache.org/r/6947/diff/3/?file=150458#file150458line136>
> >
> >     Could we move test for mixin to the mixin_test.js under the new 
> > features mixin?

This particular test was removed because these values that were in the 
container's prototype are now locally scoped objects inside the mixin closure.
The tests can't access them to manipulate them so now we only test with the 
container api.

I'd be up for moving the remaining container test to a mixin test file when we 
remove/deprecate the api from the old location.  Was trying to keep this change 
as small as possible.


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6947/#review11116
-----------------------------------------------------------


On Sept. 6, 2012, 6:03 p.m., Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6947/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2012, 6:03 p.m.)
> 
> 
> Review request for shindig, Henry Saputra, Ryan Baxter, Matt Franklin, and 
> Stanton Sievers.
> 
> 
> Description
> -------
> 
> I was running into some really weird feature dependency issues when trying to 
> resolve an issues with setTimeout and optionally supporting the common 
> container mixins with the new oauthpopup code.
> I realize we'll probably clean the oauthpopup code to only work with the 
> commoncontainer in future releases, but for this release I didn't want to 
> pull down the common container code just for this to work.
> 
> I moved the mixin functionality out to a new feature `container.mixin`.   All 
> of the old container apis point to this new feature, I didn't remove them so 
> people should not have to change any code if they use those apis.
> 
> The issue was that none of the commoncontainer code was there when I was 
> trying to add the mixin, even if it was requested (because CC depends on 
> oauthpopup, not the other way around) so I tried using a settimeout, but that 
> causes intermittent odd behavior depending on when js execution is given up 
> between eval of the script and construction of the container object.
> 
> Anyway, this seems to work well.
> 
> 
> This addresses bug SHINDIG-1864.
>     https://issues.apache.org/jira/browse/SHINDIG-1864
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/pom.xml 1380656 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.mixin/feature.xml
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.mixin/mixin.js
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js
>  1380656 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js
>  1380656 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/feature.xml
>  1380656 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/features.txt
>  1380656 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/oauthpopup/container_oauthpopup.js
>  1381205 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/oauthpopup/feature.xml
>  1380656 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js
>  1380656 
> 
> Diff: https://reviews.apache.org/r/6947/diff/
> 
> 
> Testing
> -------
> 
> Removed a test that is no longer applicable.
> 
> 
> Thanks,
> 
> Dan Dumont
> 
>

Reply via email to