shindig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Dumont" <ddum...@us.ibm.com>
Subject Re: Review Request: Change GadgetSite and UrlSite to both inherit from a common Site class, and GadgetHolder and UrlHolder to inherit from a common SiteHolder class
Date Fri, 20 Jan 2012 18:58:58 GMT


> On 2012-01-20 18:52:13, Stanton Sievers wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/lib/testutils.js,
line 143
> > <https://reviews.apache.org/r/3531/diff/2/?file=69831#file69831line143>
> >
> >     No whitespace please :)

You would bring that up in a test file, wouldn't you...   :)


> On 2012-01-20 18:52:13, Stanton Sievers wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js,
line 162
> > <https://reviews.apache.org/r/3531/diff/2/?file=69816#file69816line162>
> >
> >     Was this ever even used before?  It looks like the gadget and url sites were
generating their own ideas before this patch anyway.  Is that right?
> 
> Stanton Sievers wrote:
>     Generating their own *ids*, not *ideas*.  No sentient sites please. :)

They were both using the auto increment on the container (in case no id was provided to the
site via params or elem id), I figured it was better to move that increment to the site base
class.


- Dan


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


On 2012-01-18 20:55:12, Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3531/
> -----------------------------------------------------------
> 
> (Updated 2012-01-18 20:55:12)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The diffs did not work out how I had hoped...   But they aren't too bad.
> 
> Only a few apis actually changed names.  None of those apis were in the incubating CC
spec.
> Here are the new base classes and methods introduced:
>       <exports type="js">osapi.container.Site</exports>
>       <exports type="js">osapi.container.Site.prototype.onConstructed</exports>
>       <exports type="js">osapi.container.Site.prototype.close</exports>
>       <exports type="js">osapi.container.Site.prototype.setParentId</exports>
>       <exports type="js">osapi.container.Site.prototype.getParentId</exports>
>       <exports type="js">osapi.container.Site.prototype.getActiveSiteHolder</exports>
>       <exports type="js">osapi.container.Site.prototype.getId</exports>
>       <exports type="js">osapi.container.Site.prototype.render</exports>
>       <exports type="js">osapi.container.Site.prototype.setHeight</exports>
>       <exports type="js">osapi.container.Site.prototype.setWidth</exports>
> 
>       <exports type="js">osapi.container.SiteHolder</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.onConstructed</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.createIframeAttributeMap</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.createIframeHtml</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.dispose</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.getElement</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.getIframeElement</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.getIframeId</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.getUrl</exports>
>       <exports type="js">osapi.container.SiteHolder.prototype.render</exports>
> 
> 
> This addresses bug SHINDIG-1683.
>     https://issues.apache.org/jira/browse/SHINDIG-1683
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/cconviews.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/pom.xml 1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/feature.xml
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_holder.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/feature.xml
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_site.js
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.url/feature.xml
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.url/url_holder.js
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.url/url_site.js
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site/feature.xml
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site/site.js
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site/site_holder.js
PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.url/feature.xml
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.url/url_holder.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.url/url_site.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/feature.xml
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/features.txt
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/feature.xml
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/viewenhancements-container.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/opensearch/opensearch.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/alltests.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container.url/url_holder_test.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container.url/url_site_test.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/opensearch/opensearch_test.js
1233039 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/lib/testutils.js
1233039 
> 
> Diff: https://reviews.apache.org/r/3531/diff
> 
> 
> Testing
> -------
> 
> Fixed up tests, all tests pass.
> 
> 
> Thanks,
> 
> Dan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message