-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Make oEmbed globs configurable and extract more info from the response #10392
Conversation
Signed-off-by: Michael Telatynski <[email protected]>
A thought occurs to me (sorry for moving the goalposts): rather than embedding a list of oEmbed provider URLs, could we instead pull the Youtube's
https://oembed.com/#section7 says:
... of course that doesn't work for Twitter, so maybe we need to support a static list as well. Gah. |
Instead of maintaining our own static list, why can't we just pull in the registry from https://oembed.com/providers.json? |
I had recommended this to @t3chguy as a follow-up to keep this PR smaller. It is a bit unclear though if we need to support custom / configurable values. I can come up with use-cases for it, but I'm unsure if they're realistic or not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've written some words about how I think we should approach this over at #2752 (comment). TL;DR: let's use a providers.json
file rather than config.
Also: please could you separate the bits of this PR that aren't to do with updating the glob list out to a separate PR?
Hello! The main gist of this PR was implemented by #10714. I think the "extra more info" bits could be moved to a separate PR, but will likely be difficult to rebase on the changes. |
I'm going to close this in-lieu of #10814, which I think makes the same improvements. |
Fixes #9733
Needs moar tests, some refactoring to tidy up the code, to load the providers.json from upstream automagically
Pull Request Checklist
EventStore
toEventWorkerStore
.".code blocks
.