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

WIP ✨ Add og:video:secure_url and og:video:type #562

Merged
merged 3 commits into from
Apr 15, 2021

Conversation

TomPradat
Copy link
Contributor

Description of Change(s):

Fix #347


Some things to help get a PR reviewed and merged faster:

  • Have you updated the documentation to go with your changes?

Not yet

  • Have you written or updated unit tests?

I couldn't find tests about og:video. Should i write some ? Maybe in another PR ?

  • Have you written an integration test in the test app supplied?

Same than above

Copy link
Owner

@garmeeh garmeeh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @TomPradat. It's looking good. If you want to just add these new properties to docs that would be great.

If you could also add a check in seo.spec.js in the test below:

it('SEO overrides apply correctly', () => {

You should see some examples in there, let me know if its not clear and I can lend a hand.

@kahoowkh
Copy link
Contributor

@TomPradat I see this PR hasn't been updated for 2 months. I can help you in updating the tests and documentation.

@TomPradat
Copy link
Contributor Author

Yeap sorry, I had a busy end of year and then honestly forgot about it. I'll work on this 👍

@garmeeh
Copy link
Owner

garmeeh commented Apr 15, 2021

@all-contributors please add @TomPradat code

@allcontributors
Copy link
Contributor

@garmeeh

I've put up a pull request to add @TomPradat! 🎉

@garmeeh garmeeh merged commit e9b3a10 into garmeeh:master Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

open graph video url, secure_url and video type missing
3 participants