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

download cmd handles playlist & video #178

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

itscaro
Copy link
Contributor

@itscaro itscaro commented Apr 23, 2021

Description

download cmd now accepts a playlist ID/URL and downloads all media in the playlist.

Issues to fix

Please link issues this PR will fix:
#[issue number]

if no relevant issue, but this will fix something important for reference
, please free to open an issue.

Reminding

Something you can do before PR to reduce time to merge

  • run "make build" to build the code
  • run "make format" to reformat the code
  • run "make lint" if you are using unix system
  • run "make test-integration" to pass all tests

@itscaro itscaro changed the title download cmd handle playlist & video download cmd handles playlist & video Apr 23, 2021
@itscaro itscaro closed this Apr 29, 2021
@itscaro itscaro reopened this Apr 29, 2021
exitOnError(download(args[0]))
if playlistID, err := youtube.ExtractPlaylistID(args[0]); err != nil {
if videoID, err := youtube.ExtractVideoID(args[0]); err != nil {
exitOnError(errNotID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please try to keep the code more on the left. You don't need the else if you write instead:

videoID, err := youtube.ExtractVideoID(args[0])
if err != nil {
	exitOnError(errNotID)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@corny modifed, could you have a look?

@stemsin
Copy link

stemsin commented May 29, 2023

Can you please finish merging this PR into the master?

@corny corny force-pushed the master branch 2 times, most recently from 4da7717 to a22d61b Compare August 15, 2023 23:06
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.

3 participants