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.

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

Extend WordPress.tv embed handler to also handle VideoPress #4755

Merged
merged 9 commits into from
May 22, 2020

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented May 21, 2020

Summary

Fixes #4754.

WordPress.tv uses the VideoPress infrastructure. However, we were only directly supporting WordPress.tv but not embeds from VideoPress itself. This fixes that. Note that both WordPress.tv and VideoPress are blocks that core supports.

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter requested a review from pierlon May 21, 2020 22:48
@googlebot googlebot added the cla: yes Signed the Google CLA label May 21, 2020
@pierlon
Copy link
Contributor

pierlon commented May 21, 2020

I'll have to account for this in #4650.

@pierlon
Copy link
Contributor

pierlon commented May 21, 2020

Actually, the solution that I have does unintentionally fix this bug. Would you still consider having this merged?

@westonruter
Copy link
Member Author

We can defer to your solution instead.

@westonruter
Copy link
Member Author

Or rather, rebase this exclusively to the 1.5 branch perhaps.

@pierlon
Copy link
Contributor

pierlon commented May 21, 2020

Yes, that'd make sense.

@westonruter westonruter changed the base branch from develop to 1.5 May 21, 2020 23:00
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no Has not signed the Google CLA and removed cla: yes Signed the Google CLA labels May 21, 2020
@westonruter westonruter added cla: yes Signed the Google CLA and removed cla: no Has not signed the Google CLA labels May 21, 2020
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@westonruter
Copy link
Member Author

@pierlon This PR has exposed that builds in the 1.5 branch are failing and have been failing:

image

Could you investigate and fix?

@pierlon
Copy link
Contributor

pierlon commented May 22, 2020

Sure I'll see what the problem is.

pierlon and others added 4 commits May 22, 2020 11:39
* Fix unit tests

* Add loading attribute depending on WP version

(cherry picked from commit 4f9ac9c)
* Update spec files

* Adapt spec test to extract configuration arguments from input files

* Add STYLES configuration key to AmpRuntimeCss transformer

* Nake use of styles provided via config if available

* Remove runtime style tag if stylesheet is linked

* Add more tests to assert runtime transformer behavior

* Complete stubbed requests data

* Remove unused import

* Add git attributes file to mark certain file as being generated

* Use substr() instead of a replacement for removing the leading comment

* Remove redundant JSONOBJECT_AS_ARRAY constant

* Move .gitattributes file into lib/optimizer folder

* Revert "Move .gitattributes file into lib/optimizer folder"
* Make configuration argument optional for transformation engine

* Replace reference to Go filename

* Replace copypasta in filesystem transport

* Swap required order of configuration and remote erequest objects

* Add `null` type information where applicable

* Fix broken tests

* Use reflection to detect dependencies in correct order

* Add tests for dependency resolution

* Remove Configurable interface from schema transformer

* Fix broken test about link ordering after optimization
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no Has not signed the Google CLA and removed cla: yes Signed the Google CLA labels May 22, 2020
@pierlon
Copy link
Contributor

pierlon commented May 22, 2020

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes Signed the Google CLA and removed cla: no Has not signed the Google CLA labels May 22, 2020
@pierlon
Copy link
Contributor

pierlon commented May 22, 2020

@westonruter All seems well now. I've backported some fixes from the develop branch to fix the errors here.

@westonruter
Copy link
Member Author

Thank you very much for wrestling with this one.

@westonruter westonruter merged commit b223ef4 into 1.5 May 22, 2020
@westonruter westonruter deleted the fix/videopress branch May 22, 2020 19:50
@westonruter westonruter added this to the v1.5.4 milestone Jun 2, 2020
@westonruter westonruter linked an issue Jun 2, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Embedding VideoPress video causes validation error
3 participants