LGTM, committed.

shindig.uri is too.

On 2010/07/21 19:18:45, mhermanto wrote:
http://codereview.appspot.com/1862041/diff/9002/15004
File features/src/main/javascript/features/container/container.js
(right):

http://codereview.appspot.com/1862041/diff/9002/15004#newcode204
features/src/main/javascript/features/container/container.js:204: });
On 2010/07/21 09:45:26, johnfargo wrote:
> why not just have this method call:
> this.getGadgetMetadata(gadgetUrls, function(response) { ...stuff...
} );

Good idea on having less code, but I think service itself should deal
with more
raw/flexible request and caching/transport mechanism underneath,
instead of the
actual request itself. Example use case we may want to pass filtering
data in
request (as already supported by endpoint, not verified by me yet).
This cannot
be expressed by just gadgetUrls. Clients will not use service
directly, they
will do this via this method.

http://codereview.appspot.com/1862041/diff/9002/15004#newcode214
features/src/main/javascript/features/container/container.js:214:
gadgetUrl,
callback) {
On 2010/07/21 09:45:26, johnfargo wrote:
> ...thus changing gadgetUrl to gadgetUrls

Same reason.

http://codereview.appspot.com/1862041/diff/9002/15003
File features/src/main/javascript/features/container/gadget_holder.js
(right):

http://codereview.appspot.com/1862041/diff/9002/15003#newcode269
features/src/main/javascript/features/container/gadget_holder.js:269:
uri =
this.updateBooleanQueryParam_(uri, 'testmode');
On 2010/07/21 09:45:26, johnfargo wrote:
> feel free to use shindig.uri once committed :)
>

For sure. :)

http://codereview.appspot.com/1862041/diff/9002/15001
File features/src/main/javascript/features/container/util.js (right):

http://codereview.appspot.com/1862041/diff/9002/15001#newcode140
features/src/main/javascript/features/container/util.js:140: };
On 2010/07/21 09:45:26, johnfargo wrote:
> shindig.uri ftw

Yes. Once shindig.uri.committed.



http://codereview.appspot.com/1862041/show

Reply via email to