Skip to content

Update librespot dependency #596

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
Merged
merged 2 commits into from
Jan 28, 2021
Merged

Update librespot dependency #596

merged 2 commits into from
Jan 28, 2021

Conversation

marcelbuesing
Copy link
Contributor

@marcelbuesing marcelbuesing commented Jun 2, 2020 •
edited by mainrs
Loading

Waiting for librespot-org/librespot#485 to be merged and a new librespot version.

Furthermore I'm unsure about the naming of strings in the PLAYER_EVENT, are those coming from somewhere specifically, e.g. change, start, stop ?

For the gapless part alternatively using .. Default::default(), would have the same result, but might might make future changes non breaking (good or bad).

Edit by SirWindfield:
Fixes #85.

@ah- ah- mentioned this pull request Jun 8, 2020
Closed
@mainrs mainrs added the blocked by: librespot Issues that are blocked by librespot's upstream label Jun 11, 2020
@mainrs
Copy link
Member

mainrs commented Jun 11, 2020

I hope I do not break stuff and re-basing is the right approach here...

@mainrs mainrs mentioned this pull request Jun 11, 2020
Closed
6 tasks
@mainrs
Copy link
Member

mainrs commented Jun 11, 2020 •
edited
Loading

I am currently debating on whether I should merge it or not. Merging this means that there won't be a new release on crates.io for sure until librespot releases their new patches. But it might mean that we can squash some bugs down.

Opinions?

@backspac backspac mentioned this pull request Jun 13, 2020
Closed
@marcelbuesing
Copy link
Contributor Author

I think I would wait until this is merged in librespot.

@koiuo koiuo mentioned this pull request Jul 8, 2020
Closed
@ashthespy
Copy link

Just a heads up librespot/0.1.2 is out, but we are planning to merge in librespot-org/librespot#447 and push a bump release soon - which will probably require more churn for you, sorry!

@mainrs mainrs self-assigned this Jul 24, 2020
@sashahilton00
Copy link

sashahilton00 commented Jul 29, 2020 •
edited
Loading

v0.1.3 is out. The volume control refactor PR was included in v0.1.3, so watch out for a small breaking change in that --linear-volume has changed to --volume-ctrl with a few options. @ashthespy if you could document those and the cubic mixer PR in the wiki at some point, that'd be great.

@marcelbuesing marcelbuesing changed the title [WIP] Update librespot dependency Update librespot dependency Aug 18, 2020
@marcelbuesing
Copy link
Contributor Author

I think this could be merged now. Please review again.

@Barbaross93
Copy link

Just a heads-up, line 83 of setup.rs in the bump-librespot branch has a misspelled VolumeCtrl. Currently it's VolumeCtl.

@marcelbuesing marcelbuesing force-pushed the bump-librespot branch 2 times, most recently from ac3be44 to 37cc041 Compare September 25, 2020 09:30
@marcelbuesing
Copy link
Contributor Author

Just a heads-up, line 83 of setup.rs in the bump-librespot branch has a misspelled VolumeCtrl. Currently it's VolumeCtl.

Finally fixed =)

@codicodi
Copy link

I've been using this PR for a while and it seems to somehow break MPRIS play-pause functionality. playerctl play-pause only can start playback but cannot pause it anymore.

@JojiiOfficial JojiiOfficial mentioned this pull request Oct 22, 2020
Closed
@codygman codygman mentioned this pull request Dec 18, 2020
Open
@stale
Copy link

stale bot commented Jan 13, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix Issues that will not be fixed under any circumstances label Jan 13, 2021
@stale stale bot removed the wontfix Issues that will not be fixed under any circumstances label Jan 23, 2021
@marcelbuesing
Copy link
Contributor Author

I just rebased this PR on top of the latest changes in spotifyd, I'm confident that librespot might soon support tokio 1.0 so it would probably be good to update to librespot 0.1.3 before the next major breaking change.

@slondr slondr added the pinned Apply to make stale bot ignore issue/PR. label Jan 24, 2021
@slondr slondr added this to the v0.4.0 milestone Jan 24, 2021
@robinvd
Copy link
Contributor

robinvd commented Jan 25, 2021 •
edited
Loading

Amazing to hear they might update to 1.0!, ill look at this pr soon.

EDIT: looks good, ill test it during the day tomorrow

robinvd
robinvd previously approved these changes Jan 26, 2021
View reviewed changes
SimonTeixidor
SimonTeixidor previously approved these changes Jan 26, 2021
View reviewed changes
@robinvd robinvd dismissed stale reviews from SimonTeixidor and themself via ae997b1 January 27, 2021 11:15
@robinvd robinvd requested a review from SimonTeixidor January 28, 2021 13:05
@robinvd robinvd merged commit 5175eb8 into Spotifyd:master Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimonTeixidor SimonTeixidor SimonTeixidor approved these changes

@robinvd robinvd robinvd approved these changes

@JojiiOfficial JojiiOfficial Awaiting requested review from JojiiOfficial

Assignees

@mainrs mainrs

Labels
blocked by: librespot Issues that are blocked by librespot's upstream pinned Apply to make stale bot ignore issue/PR.
Projects
None yet
Milestone
v0.4.0
Development

Successfully merging this pull request may close these issues.

Gapless support
9 participants
@marcelbuesing @mainrs @ashthespy @sashahilton00 @Barbaross93 @codicodi @robinvd @SimonTeixidor @slondr