Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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.

🚀

@slondr slondr requested a review from a team March 9, 2023 00:46
@eladyn eladyn mentioned this pull request Mar 9, 2023
@eladyn eladyn force-pushed the rspotify_id_fixes branch from 69eecea to ffa43c2 Compare March 9, 2023 19:56
@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 eladyn force-pushed the rspotify_id_fixes branch from ac0ff40 to d5b3fd0 Compare March 16, 2023 23:59
@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
@eladyn eladyn deleted the rspotify_id_fixes branch March 22, 2023 01:12
# for free to join this conversation on GitHub. Already have an account? # 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