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

remove workaround for rspotify id types #1145

Merged
merged 2 commits into from Mar 22, 2023

Conversation

eladyn
Copy link
Member

@eladyn eladyn commented Dec 17, 2022

This upgrades rspotify to 0.11.6 and removes the workaround that was introduced in #1079, for which there is now much better library support.

related: #1144

slondr
slondr previously approved these changes Dec 20, 2022
Copy link
Member

@slondr slondr left a comment

Choose a reason for hiding this comment

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

cool!

@eladyn eladyn requested a review from a team December 20, 2022 06:47
@eladyn eladyn mentioned this pull request Mar 2, 2023
@eladyn eladyn requested review from slondr and a team and removed request for a team March 6, 2023 20:16
@eladyn
Copy link
Member Author

eladyn commented Mar 6, 2023

I rebased the PR on current master. Maybe we can get this merged soon-ish, such that CI passes again?

@Icelk
Copy link
Contributor

Icelk commented Mar 6, 2023

@eladyn maybe we should add --locked in CI? That fixes the CI, since it uses the versions from Cargo.lock

@eladyn
Copy link
Member Author

eladyn commented Mar 7, 2023

@Icelk That might indeed be a good idea. Well, the effort of fixing CI failures is – in theory – not that big. It's only becoming a hurdle, when other CI for other PRs are failing due to the fix not yet being merged.

slondr
slondr previously approved these changes Mar 9, 2023
Copy link
Member

@slondr slondr left a comment

Choose a reason for hiding this comment

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

🚀

@eladyn
Copy link
Member Author

eladyn commented Mar 9, 2023

@slondr Sorry for dismissing your review once again. GitHub was acting very strangely and somehow told me that there were still conflicts even though I remember resolving them some days ago? I now resolved them manually and hopefully, this time everything should be fine.

@eladyn eladyn mentioned this pull request Mar 9, 2023
@eladyn eladyn force-pushed the rspotify_id_fixes branch 3 times, most recently from 5daffe0 to c4001da Compare March 10, 2023 23:24
@eladyn
Copy link
Member Author

eladyn commented Mar 11, 2023

Ugh, somehow it now suddenly wants to use time@0.3.20, which requires an MSRV bump. Should I do that here or should we just merge this and try to get the MSRV bump in a different PR as soon as possible?

@Icelk
Copy link
Contributor

Icelk commented Mar 11, 2023

I'll finish up #1182 today, which includes the MSRV bump. It would block on this, so if we merge the two right after each other, everything should work out.

@slondr slondr added this to the v0.3.5 milestone Mar 11, 2023
@eladyn
Copy link
Member Author

eladyn commented Mar 17, 2023

Now, we should (finally) be fine!

@eladyn eladyn requested a review from slondr March 17, 2023 00:00
@eladyn eladyn enabled auto-merge March 17, 2023 00:09
@eladyn eladyn mentioned this pull request Mar 21, 2023
@eladyn eladyn mentioned this pull request Mar 21, 2023
6 tasks
@eladyn eladyn linked an issue Mar 21, 2023 that may be closed by this pull request
6 tasks
@slondr
Copy link
Member

slondr commented Mar 22, 2023

aight let's send it yeet

@eladyn eladyn merged commit 9932391 into Spotifyd:master Mar 22, 2023
6 checks passed
@eladyn eladyn deleted the rspotify_id_fixes branch March 22, 2023 01:12
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.

Cannot Build with dbus_mpris
3 participants