Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(483)

Issue 3842042: Support preloading gadgets metadata as part of container config

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 2 months ago by zhoresh
Modified:
15 years, 2 months ago
Reviewers:
johnfargo, dev-remialer, mhermanto
Base URL:
http://svn.apache.org/repos/asf/shindig/trunk
Visibility:
Public.

Description

Add support for in-lining of gadgets metadata as part of the container page in order to save another round-trip to server to get metadata separately.

Patch Set 1 #

Total comments: 12
Unified diffs Side-by-side diffs Delta from patch set Stats (+156 lines, -19 lines) Patch
features/src/main/javascript/features/container/container.js View 4 chunks +62 lines, -11 lines 5 comments Download
features/src/main/javascript/features/container/service.js View 2 chunks +54 lines, -8 lines 6 comments Download
features/src/test/javascript/features/container/container_test.js View 1 chunk +11 lines, -0 lines 1 comment Download
features/src/test/javascript/features/container/service_test.js View 1 chunk +29 lines, -0 lines 0 comments Download

Messages

Total messages: 5
zhoresh
15 years, 2 months ago (2011-01-05 01:21:47 UTC) #1
mhermanto
On 2011/01/05 01:21:47, zhoresh wrote: LGTM++. Mostly nits, please fix those before submitting.
15 years, 2 months ago (2011-01-07 02:32:55 UTC) #2
mhermanto
http://codereview.appspot.com/3842042/diff/1/features/src/main/javascript/features/container/container.js File features/src/main/javascript/features/container/container.js (right): http://codereview.appspot.com/3842042/diff/1/features/src/main/javascript/features/container/container.js#newcode364 features/src/main/javascript/features/container/container.js:364: shindig.container.ContainerConfig.PRELOAD_TOKENS = 'preloadTokens'; Major nit on plural, PRELOAD_TOKEN or ...
15 years, 2 months ago (2011-01-07 02:33:09 UTC) #3
zhoresh
Thanks for the review, all comments were addressed and code is committed. On Thu, Jan ...
15 years, 2 months ago (2011-01-07 18:17:08 UTC) #4
johnfargo
15 years, 2 months ago (2011-01-07 22:51:53 UTC) #5
LGTM++

http://codereview.appspot.com/3842042/diff/1/features/src/main/javascript/fea...
File features/src/main/javascript/features/container/container.js (right):

http://codereview.appspot.com/3842042/diff/1/features/src/main/javascript/fea...
features/src/main/javascript/features/container/container.js:352:
shindig.container.ContainerConfig.PRELOAD_REF_TIME = 'preloadRefTime';
nit for a later time: I'd personally prefer to get rid of these long-form
"constants"
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b