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.

Sign up for GitHub

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

Jump to bottom

Fix #1044 #1064

Merged
merged 2 commits into from Jan 6, 2021
Merged

Fix #1044 #1064

merged 2 commits into from Jan 6, 2021

Conversation

pithuene
Copy link
Contributor

Updates the expected return value of the YTM query.

pithuene
pithuene commented Dec 30, 2020
View reviewed changes
spotdl/search/songObj.py
@@ -70,8 +70,6 @@ def from_url(cls, spotifyURL: str):
duration
)

youtubeLink = youtubeLink
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope this was as unnecessary as it looks and not some hidden operator overloading magic or something like that

Copy link

Choose a reason for hiding this comment

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

I think its unnecessary too.

@pithuene pithuene force-pushed the master branch 2 times, most recently from d0aec10 to 99de46f Compare December 30, 2020 14:20
@Silverarmor Silverarmor linked an issue Jan 2, 2021 that may be closed by this pull request
Playlist Download error - TypeError: remove #1044
Closed
aklajnert
aklajnert approved these changes Jan 3, 2021
View reviewed changes
Copy link
Contributor

@aklajnert aklajnert left a comment

Choose a reason for hiding this comment

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

The change from list to dict is a great improvement for readability, as it removed the magic numbers.

You should also change the target branch to dev, and apply the modification from your other PR (print artist and song name instead of just the link).

@pithuene
Fix spotDL#1044
4d25594 
Updates the expected return value of the YTM query.
@pithuene pithuene changed the base branch from master to dev January 5, 2021 10:05
@pithuene
Copy link
Contributor Author

pithuene commented Jan 5, 2021

The change from list to dict is a great improvement for readability, as it removed the magic numbers.

You should also change the target branch to dev, and apply the modification from your other PR (print artist and song name instead of just the link).

Done

@pithuene pithuene mentioned this pull request Jan 5, 2021
Closed
@Silverarmor
Copy link
Member

Should this be being merged into spotdl:master rather than the dev? - Since it fixes a bug that is currently present in master...

@aklajnert
Copy link
Contributor

I think it should be merged into dev then dev into master.

@Silverarmor
Copy link
Member

@pithuene Can you just confirm that I resolved that conflict correctly?
Took Line 183 from #1065.

@pithuene
Copy link
Contributor Author

pithuene commented Jan 6, 2021

Looks good to me 👍

@Silverarmor Silverarmor merged commit 3297df3 into spotDL:dev Jan 6, 2021
@Silverarmor Silverarmor added the Bug Fix PRs that fix bugs label Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aklajnert aklajnert aklajnert approved these changes

Assignees
No one assigned
Labels
Bug Fix PRs that fix bugs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Playlist Download error - TypeError: remove
3 participants
@pithuene @Silverarmor @aklajnert