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

Issue 10041: Client side Data Pipelining in OS Templates

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 3 months ago by levik
Modified:
9 years, 3 months ago
Reviewers:
shindig-dev, Evan Gilbert
CC:
levik
Base URL:
http://svn.apache.org/repos/asf/incubator/shindig/trunk/
Visibility:
Public.

Description

Overhauled the implementation of Data Pipelining in Templates. <os:ViewerRequest>, <os:OwnerRequest>, <os:PeopleRequest> (@userId and @groupId only for now) and <os:DataRequest> (@href and @format=json|text only for now). Supports automatic dependency detection and chaining TODOs: * support all proposed tags, attributes * remove dependency on templates * move into own feature, or core. * add more Unit tests JIRA Issue: https://issues.apache.org/jira/browse/SHINDIG-762

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+495 lines, -358 lines) Patch
features/opensocial-templates View 0 chunks +-1 lines, --1 lines 0 comments Download
features/opensocial-templates/base.js View 1 chunk +2 lines, -1 line 1 comment Download
features/opensocial-templates/compiler.js View 5 chunks +5 lines, -3 lines 2 comments Download
features/opensocial-templates/container.js View 6 chunks +42 lines, -37 lines 2 comments Download
features/opensocial-templates/data.js View 8 chunks +420 lines, -277 lines 4 comments Download
features/opensocial-templates/data_test.js View 2 chunks +10 lines, -16 lines 0 comments Download
features/opensocial-templates/ost_test.html View 2 chunks +4 lines, -8 lines 0 comments Download
features/opensocial-templates/template_test.js View 1 chunk +3 lines, -1 line 2 comments Download
java/server/src/test/resources/endtoend/opensocial-templates/data_test.js View 2 chunks +10 lines, -16 lines 0 comments Download

Messages

Total messages: 2
levik
15 years, 3 months ago (2008-12-04 19:19:13 UTC) #1
Evan Gilbert
15 years, 3 months ago (2008-12-12 19:27:14 UTC) #2
Made a number of small edits to the patch before submitting.

The only open items are:
- We should change constants to ALL_CAPS
- os.data.getDataContext().putDataSet() is quite verbose and we should possibly
propose spec alternatives.

http://codereview.appspot.com/10041/diff/1/7
File features/opensocial-templates/base.js (right):

http://codereview.appspot.com/10041/diff/1/7#newcode94
Line 94: variableSubstitution: /^([^$]*)(\$\{[^\}]*\})([\w\W]*)$/
Should constants be ALL_CAPS

http://codereview.appspot.com/10041/diff/1/8
File features/opensocial-templates/compiler.js (right):

http://codereview.appspot.com/10041/diff/1/8#newcode176
Line 176: 'Count': VAR_count,
Removing this "," - it will break IE

http://codereview.appspot.com/10041/diff/1/8#newcode743
Line 743: console.log(child);
Removing this and the following log statement, I assume these are debugging
artefacts

http://codereview.appspot.com/10041/diff/1/9
File features/opensocial-templates/container.js (right):

http://codereview.appspot.com/10041/diff/1/9#newcode194
Line 194: var beforeData = node.getAttribute('before');
Changing to support both for backwards compatibility. We should update demos and
check with current clients of the pre-release templates before removing.

http://codereview.appspot.com/10041/diff/1/9#newcode202
Line 202: var requiredData = node.getAttribute('require');
ditto

http://codereview.appspot.com/10041/diff/1/5
File features/opensocial-templates/data.js (right):

http://codereview.appspot.com/10041/diff/1/5#newcode69
Line 69: // TODO(levik): This attribute may not be used by the handler.
Removed name on TODO, here an in places that were around already.

http://codereview.appspot.com/10041/diff/1/5#newcode124
Line 124: os.data.RequestDescriptor.prototype.getSendRequestClosure = function()
{
We should add a bind function - these callbacks will become clearer.

http://codereview.appspot.com/10041/diff/1/5#newcode299
Line 299: os.data.getDataContext = function() {
Spec issue - this is very verbose. I'm going to propose a spec change to make
this
os.data.put() and
os.data.get()

http://codereview.appspot.com/10041/diff/1/5#newcode475
Line 475: gadgets.util.registerOnLoadHandler(os.data.processDocumentMarkup);
This was breaking standalone unit tests as gadgets was not defined - wrapped it
in if (window['gadgets'] && window['gadgets']['util']) {}

http://codereview.appspot.com/10041/diff/1/6
File features/opensocial-templates/template_test.js (right):

http://codereview.appspot.com/10041/diff/1/6#newcode353
Line 353: //os.createNamespace("custom", "#");
Removed this line

http://codereview.appspot.com/10041/diff/1/6#newcode357
Line 357: //assertEquals('helloworld', domutil.getVisibleText(output));
Reenabled this line
Sign in to reply to this message.

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