[peloton] Add Peloton as new extractor #192
Conversation
I see the code is taken from ytdl-org/youtube-dl#24985 |
|
related: ytdl-org/youtube-dl#21853 |
What's the procedure for adding code from youtube-dl and youtube-dlc, especially if the code has to be fixed before those programs merge the updates? |
|
Since all PRs made to youtube-dl/dlc are already unlicensed, there is no issue with merging them here. But explain in the PR description where it is taken from and clarify whether you have made modifications. That way I can give proper credits and also have easy reference to the original PR/issue. Also don't tick "I am the original author" when you are not! |
If you've made modifications, you'd have to mark both checkboxes. Maybe the guide should have an additional checkbox for modified unlicensed code, and a note to reference the original PR/issue. |
I have updated the PR description and provided additional description of where the code was from as well as the related issues it came from on youtube-dl. As it was also apart of the bulk request for evaluation of new extractors ytdl-org/youtube-dl#28054 |
|
@IONECarter Are you familiar with the actual code of this PR? I am asking to know whether you will be able to make changes if I review or I need to make any necessary changes myself Also, since I cannot test this, please run all the tests yourself and confirm that all the cases pass pinging @capntrips just to let them know of this |
|
I'm just happy to see that it might get some use, since it went nowhere on youtube-dl. |
I have tested this with incorporating it into my local version of 2021.03.21 and it worked for a number of videos that I tested as of yesterday when the PR was made. I've looked over the code to familiarize myself with it when I was testing it to see if any updates were required to get it to work (none were needed), but may require additional help from yourself or @capntrips should you have significant updates wanted. |
What I meant is that you need to run the test cases defined in the extractor. Or maybe @capntrips can confirm that all the tests are working and up to date? |
Yes, I've performed all three test cases that are noted in the extractor and all are working and up to date. |
|
Still here and working through. Will provide code updates & comment responses once I am comfortable with it. Learning as I go :) |
pukkandan
force-pushed
the
master
branch
2 times, most recently
from
2502e40
to
2305e2e
Compare
|
heads up for anyone trying to get this branch working: |
pukkandan
force-pushed
the
master
branch
2 times, most recently
from
162dd51
to
da503b7
Compare
Thank you very much for posting this. I have not be comfortable enough with my coding to get the above comments addressed. Not sure if you want to look at it @delikat or if @capntrips is still around, but would be great if this was able to get merged in for a broader use. |
@IONECarter You should have said so. I can address most of the points myself. I didn't bother with it since you said you'd do it |
Many, many thanks. I was really hoping that I could learn and teach myself, but that has not been going as well as I had hoped and I have not had near enough time to really dig in. You're the BEST! |
|
I pulled the PR and made some changes, but then realized that an account is needed for all content. Please provide me with account details if you want me to work on this |
Please follow the guide below
xinto all the boxes [ ] relevant to your pull request (like that [x])Before submitting a pull request make sure you have:
In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:
What is the purpose of your pull request?
Description of your pull request and other information
Adds an extractor for Peloton (onepeloton.com) live and on-demand workout videos and audio recordings. An account is needed to download all media.
As referenced below original code pulled over from ytdl-org/youtube-dl#24985 where code was submitted as a PR to youtube-dl but was not merged. Part of overall issue ytdl-org/youtube-dl#28054 where there are a number of new/updated extractors that need testing prior to merge.