Add o2player extension #3305
Add o2player extension #3305
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
| <h2>O2Player</h2> | ||
|
|
||
| <amp-o2player | ||
| data-pid="573d9bf1e4b02a3388f42c36" |
|
Thanks for the extensive docs! A few comments above. |
|
@yevheniiminin Is this ready for another round of review? |
|
@rudygalfi Yes, we fixed comments and travis build |
| const macros = this.element.getAttribute('data-macros'); | ||
| const env = this.element.getAttribute('data-env'); | ||
| user.assert( | ||
| (pid && bcid) || vid, |
| @@ -0,0 +1,103 @@ | |||
| <!--- | |||
There was a problem hiding this comment.
Please link in README.md one level up.
|
LGTM with one comment. @Gregable Would you mind taking a look at the validator changes? Thanks! |
|
Quick ping @Gregable |
|
ping @Gregable -- did you get a chance to look at the validator changes @cramforce mentioned? |
| # | ||
|
|
||
| tags: { # amp-o2-player | ||
| tag_name: "script" |
There was a problem hiding this comment.
Due to changes since you started this PR, tag_name fields are now upper case. Can you please change this to:
tag_name: "SCRIPT"
|
Apologies for missing this thread somehow. I've added a few comments to the validator changes. Thanks for making these! |
|
Thanks for the changes. Validation looks good to me. |
|
LGTM |
|
FYI, the validations changes for this are now live everywhere. |
No description provided.