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

Add peertube #5065

Merged
merged 3 commits into from Feb 7, 2019
Merged

Add peertube #5065

merged 3 commits into from Feb 7, 2019

Conversation

@aliceinwire
Copy link
Contributor

aliceinwire commented Jan 31, 2019

Adding peertube

@aliceinwire aliceinwire force-pushed the aliceinwire:alicef/peertube branch 3 times, most recently from 49e18ab to 508bd19 Feb 1, 2019
@aliceinwire aliceinwire changed the title WIP: Add pertube Add pertube Feb 1, 2019
@aliceinwire aliceinwire changed the title Add pertube Add peertube Feb 2, 2019
@aliceinwire aliceinwire force-pushed the aliceinwire:alicef/peertube branch 3 times, most recently from b2ef015 to ea63eb0 Feb 2, 2019
@aliceinwire

This comment has been minimized.

Copy link
Contributor Author

aliceinwire commented Feb 2, 2019

@BenMcGarry BenMcGarry added RE-MediaHost and removed Status-1_WIP labels Feb 2, 2019
@BenMcGarry

This comment has been minimized.

Copy link
Collaborator

BenMcGarry commented Feb 2, 2019

Thanks for the contribution!

Tested and confirmed working on Chrome + Edge. Firefox confirmed based on above screenshot.

@larsjohnsen Do we need the migration trigger as this is a new module?

Related to #5049, https://peervideo.net/about/peertube looks like the best URL for that PR, the multiple domains are covered by it.

@aliceinwire

This comment has been minimized.

Copy link
Contributor Author

aliceinwire commented Feb 2, 2019

Would be nice to add a setting in the UI for adding new domain for Peertube.
But I have no idea if is possible or accepted.
Anyway it will probably be in a next pull request, as I'm already enough out of my competence sphere.

lib/core/migrate/migrate.js Outdated Show resolved Hide resolved
@aliceinwire aliceinwire force-pushed the aliceinwire:alicef/peertube branch from ea63eb0 to 8bbbba1 Feb 2, 2019
@aliceinwire

This comment has been minimized.

Copy link
Contributor Author

aliceinwire commented Feb 2, 2019

Leaved untouched migrate.js and expand only with videos.
tested here:
https://screenshots.firefox.com/51zDsLk99btKVqYA/www.reddit.com

@aliceinwire

This comment has been minimized.

Copy link
Contributor Author

aliceinwire commented Feb 2, 2019

@kevinji thanks

@SoniEx2

This comment has been minimized.

Copy link

SoniEx2 commented Feb 3, 2019

any reason this uses a domain whitelist?

@aliceinwire

This comment has been minimized.

Copy link
Contributor Author

aliceinwire commented Feb 3, 2019

@SoniEx2
peertube instance url depend from the server.
As now I'm adding the most common one.
I have some idea to implement in the RES settings, maybe in a next pull request.

@Nutomic

This comment has been minimized.

Copy link

Nutomic commented Feb 6, 2019

Maybe you could check for the Peertube meta tag instead of hardcoding an instance list? However it is not released yet.

@SoniEx2

This comment has been minimized.

Copy link

SoniEx2 commented Feb 6, 2019

it's actually possible to detect the URL layouts of peertube and the peertube API. altho the tag does make it easier I guess.

@larsjohnsen

This comment has been minimized.

Copy link
Collaborator

larsjohnsen commented Feb 7, 2019

@Nutomic That would require loading all links, correct? I think that would be infeasible, both due to bandwidth and privacy concerns.

@SoniEx2

This comment has been minimized.

Copy link

SoniEx2 commented Feb 7, 2019

Hm. p2p-sourced link data?

@bjuergens

This comment has been minimized.

Copy link
Contributor

bjuergens commented Feb 7, 2019

This may be a little off-topic, but shouldn't the og-tags always be loaded for unkown urls? After all og:video and og:image exists exactly for situations like this.

@aliceinwire

This comment has been minimized.

Copy link
Contributor Author

aliceinwire commented Feb 7, 2019

@SoniEx2 @Nutomic the problem is doing it from just the URL.
If you have a better way please send fix commit here.

Alice Ferrazzi and others added 2 commits Jan 31, 2019
@aliceinwire aliceinwire force-pushed the aliceinwire:alicef/peertube branch from ec0f8cf to a7c7a74 Feb 7, 2019
@SoniEx2

This comment has been minimized.

Copy link

SoniEx2 commented Feb 7, 2019

that pissed me off with youtube-dl and it pisses me off here too. altho youtube-dl downloads the full page anyway so they don't have an excuse for it. does RES not?

@larsjohnsen larsjohnsen merged commit 40436d1 into honestbleeps:master Feb 7, 2019
3 of 4 checks passed
3 of 4 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.0004%) to 1.443%
Details
security/snyk - package.json (honestbleeps (GitHub marketplace)) No new issues
Details
@larsjohnsen

This comment has been minimized.

Copy link
Collaborator

larsjohnsen commented Feb 7, 2019

Thanks, @aliceinwire!

@aliceinwire aliceinwire deleted the aliceinwire:alicef/peertube branch Feb 8, 2019
@aliceinwire

This comment has been minimized.

Copy link
Contributor Author

aliceinwire commented Feb 10, 2019

would be nice to use a peertube instance list like https://fediverse.network/about
unfortunatly their api is not ready yet

@SoniEx2

This comment has been minimized.

Copy link

SoniEx2 commented Feb 10, 2019

I don't think that handles non-federating instances

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.