Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ [amp-render] implement xssi-prefix, sub-object and refresh support #33894

Merged
merged 24 commits into from
Apr 21, 2021
Merged

✨ [amp-render] implement xssi-prefix, sub-object and refresh support #33894

merged 24 commits into from
Apr 21, 2021

Conversation

dmanek
Copy link
Contributor

@dmanek dmanek commented Apr 19, 2021 •
edited

Partial for #32477

@dmanek dmanek changed the title ✨ [amp-render] xssi-prefix, expr and refresh support ✨ [amp-render] implement xssi-prefix, sub-object and refresh support Apr 19, 2021
@dmanek dmanek marked this pull request as ready for review April 19, 2021 22:07
@dmanek dmanek requested review from caroqliu and krdwan April 19, 2021 22:08
caroqliu
caroqliu reviewed Apr 20, 2021
View reviewed changes
extensions/amp-render/1.0/amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/amp-render.js Show resolved Hide resolved
extensions/amp-render/1.0/test/test-amp-render.js Show resolved Hide resolved
extensions/amp-render/1.0/test/test-amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/test/test-amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/test/test-amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/test/test-amp-render.js Outdated Show resolved Hide resolved
alanorozco
alanorozco reviewed Apr 20, 2021
View reviewed changes
extensions/amp-render/1.0/amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/amp-render.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/amp-render.js Show resolved Hide resolved
dmanek and others added 8 commits April 20, 2021 09:20
@dmanek @caroqliu
Update extensions/amp-render/1.0/amp-render.js
3c12f07 
Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
@dmanek @caroqliu
Update extensions/amp-render/1.0/amp-render.js
ecab9b0 
Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
@dmanek @caroqliu
Update extensions/amp-render/1.0/amp-render.js
9f8fc64 
Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
@dmanek @caroqliu
Update extensions/amp-render/1.0/test/test-amp-render.js
b29eedd 
Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
@dmanek @caroqliu
Update extensions/amp-render/1.0/test/test-amp-render.js
f27742a 
Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
@dmanek @caroqliu
Update extensions/amp-render/1.0/test/test-amp-render.js
03a82de 
Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
@dmanek
WIP
742bddf
@dmanek
WIP
cbf9c97
krdwan
krdwan approved these changes Apr 20, 2021
View reviewed changes
Copy link
Contributor

@krdwan krdwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending comments from Alan and Caroline!

caroqliu
caroqliu approved these changes Apr 21, 2021
View reviewed changes
Copy link
Contributor

@caroqliu caroqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job!!

extensions/amp-render/1.0/test/test-amp-render.js Outdated Show resolved Hide resolved
caroqliu
caroqliu reviewed Apr 21, 2021
View reviewed changes
src/batched-json.js Outdated Show resolved Hide resolved
alanorozco
alanorozco approved these changes Apr 21, 2021
View reviewed changes
Copy link
Member

@alanorozco alanorozco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

src/batched-json.js Outdated Show resolved Hide resolved
src/batched-json.js Outdated Show resolved Hide resolved
dmanek and others added 5 commits April 21, 2021 09:22
@dmanek @caroqliu
Update extensions/amp-render/1.0/test/test-amp-render.js
7d7629d 
Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
@dmanek @caroqliu
Update src/batched-json.js
a7151d7 
Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
@dmanek @alanorozco
Update src/batched-json.js
05abf7c 
Co-authored-by: Alan Orozco <alanorozco@users.noreply.github.com>
@dmanek
Merge branch 'amp-render-xssi-prefix' of github.com:dmanek/amphtml in…
01459cc 
…to amp-render-xssi-prefix
@dmanek
WIP
9d7edd2
@dmanek dmanek requested a review from samouri April 21, 2021 16:32
samouri
samouri approved these changes Apr 21, 2021
View reviewed changes
Copy link
Member

@samouri samouri left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nits/questions

extensions/amp-render/1.0/test/test-amp-render.js Show resolved Hide resolved
extensions/amp-render/1.0/test/test-amp-render.js Show resolved Hide resolved
@dmanek dmanek merged commit 0c4e5aa into ampproject:main Apr 21, 2021
@dmanek dmanek deleted the amp-render-xssi-prefix branch April 21, 2021 17:13
rochapablo pushed a commit to rochapablo/amphtml that referenced this pull request Aug 30, 2021
@dmanek @rochapablo
✨ [amp-render] implement xssi-prefix, sub-object and refresh support (a…
7d61874 
…mpproject#33894)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caroqliu caroqliu caroqliu approved these changes

@alanorozco alanorozco alanorozco approved these changes

@samouri samouri samouri approved these changes

@krdwan krdwan krdwan approved these changes

Assignees
No one assigned
Labels
PR use: In Beta / Experimental PR use: In Stable
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@dmanek @alanorozco @samouri @caroqliu @krdwan @ampprojectbot