On 2009/10/20 21:53:57, johnfargo wrote:
Aside from the rather minor comments below, my primary concern is that
this adds
bulk to every gadget render, not just those that are cajoled.
This said, I agree it's the right move to include tamings with
features
themselves. It appears that tamings for gadget and container context
are
equivalent - if so, a new RenderingContext.CAJA might be in order: <tamings> <script src="..."/> </tamings>
Or we could try to genericize this by adding extensible syntax: <ext:caja> <script src="..."/'> </ext:caja> ...the content of which could be read by a Rewriter
(CajaContentRewriter) and
injected as needed.
...or something similar. Thoughts?
jas...@rorohiko:~/src/svn-changes/shindig-tame/features/src/main/javascript/features:0$ for i in */taming.js ; do x=$(du -b `dirname $i` --exclude=.svn); echo `du -b $i | cut -f1` $x; done 3676 10135 caja 1096 22998 core.io 2006 60921 core 1087 12984 dynamic-height 5413 17379 flash 1273 14395 minimessage 6389 151840 opensocial-reference 1105 12448 pubsub 1052 7408 settitle 1034 9058 skins 1578 27632 tabs 1381 17326 views Do you think the gains are significant in comparison to the size of each feature and the complexity of maintaining another renderingcontext?
http://codereview.appspot.com/135051/diff/1027/48 File features/src/main/javascript/features/caja/taming.js (right):
http://codereview.appspot.com/135051/diff/1027/48#newcode105 Line 105: var tamings___ = tamings___ || []; Not that it's a big deal in this case, but maybe it should be. This is
one of a
few use cases I've seen arise that call for a clearer representation
of the
feature dependency tree.
http://codereview.appspot.com/135051/diff/1027/48#newcode115 Line 115: ___.grantFunc(imports.outers.console, 'log'); nit: I wonder if this is necessary, vs. just encouraging people to use gadgets.log.
http://codereview.appspot.com/135051/diff/1027/46 File features/src/main/javascript/features/flash/taming.js (right):
http://codereview.appspot.com/135051/diff/1027/46#newcode1 Line 1: /* Missing a corresponding feature.xml update for flash.
http://codereview.appspot.com/135051/diff/1027/46#newcode36 Line 36: var O = ifr.contentWindow.Object; possible mini-optimization: pull A and O above var cleanse to avoid
having to
recreate the frame.
http://codereview.appspot.com/135051/diff/1027/54 File
features/src/main/javascript/features/opensocial-jsonrpc/jsonrpccontainer.js
(right):
http://codereview.appspot.com/135051/diff/1027/54#newcode59 Line 59: var JsonRpcRequestItem = function(rpc, opt_processData) { nit: spacing 2 to the left.
http://codereview.appspot.com/135051
