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

Future breaking update of Rspotify #692

Closed
marioortizmanero opened this issue Oct 18, 2020 · 7 comments
Closed

Future breaking update of Rspotify #692

marioortizmanero opened this issue Oct 18, 2020 · 7 comments
Labels
discussion enhancement A new feature that would improve Spotifyd refactor Code changes that do not effect the functionality wontfix Issues that will not be fixed under any circumstances

Comments

@marioortizmanero
Copy link

marioortizmanero commented Oct 18, 2020

Hello!

I'm a maintainer of Rspotify. I picked up this library in July and have made lots of improvements since, with the help of the original maintainer. The library is now much cleaner, more flexible, and more performant. For now, these are the noticeable improvements for the end user:

  • async code has dropped from 206 (!!) compilation units (~1 min 29s on my machine) to 154 (~1min 10s)
  • blocking code has dropped from 207 compilation units (~1min 34s) to 103 (~1 min 1s) and now uses ureq
  • from 16310 LoC to 7268 (obtained with Tokei)
  • more clear documentation, getting started should be more straightforward
  • i intend to make an actual benchmark with more details

And we are just at 50% of the changes we intend to make, so it'll be even better. But these kind of improvements require breaking changes. A lot of them, in fact. I wanted to ask the maintainers of this crate (which is one of the most active ones using Rspotify, although you seem to be using an outdated version, check this out) for help in some discussions relating the future of Rspotify:

Before the new release, we'd like to at least finish these changes, and afterwards we can keep adding new features and prepare for v1.0 in the far future. These include lots of proposals and changes, so don't feel the need to check out each of them. At least a bit of feedback will be useful to us.

Edit: this issue might be mislabeled.

@marioortizmanero marioortizmanero added the bug A functionality or parts of a program that do not work as intended label Oct 18, 2020
@JojiiOfficial JojiiOfficial added discussion enhancement A new feature that would improve Spotifyd refactor Code changes that do not effect the functionality and removed bug A functionality or parts of a program that do not work as intended labels Oct 18, 2020
@robinvd
Copy link
Contributor

robinvd commented Nov 14, 2020

@marioortizmanero i just tried the current master branch of rspotify, and it seems to work without a lot of change. What is the timeline for releasing the version on crates.io? so that we can update to the newest version.

as for the breaking changes, this project only uses a few basic api requests like the current song. So i think most breaking changes wont effect us or woudnt need too much work on our side.

edit: link to diff robinvd@a863582

@marioortizmanero
Copy link
Author

We don't have a set release date but I can tell you there's still a lot of work left, hopefully we can release before 2021 but I can't assure that. We think it's better to release all breaking changes in a single version instead of multiple ones (do you agree?).

It's great that you already tried to update the code. In your diff I can see some functions that won't be needed after we're done, like datetime_to_timestamp (we'll use datetimes instead of timestamps) and is_token_expired (this is now Token::is_expired).

@robinvd
Copy link
Contributor

robinvd commented Nov 16, 2020

We think it's better to release all breaking changes in a single version instead of multiple ones (do you agree?)

yes i agree, maybe if breaking changes are unrelated they could be in a different breaking release, but in general fewer is better. Then ill personally wait for the release before making a pr so i dont have to upgrade twice (but if anyone else wants to update to the currently latest crates.io version, feel free).

It's great that you already tried to update the code. In your diff I can see some functions that won't be needed after we're done

that sounds like a great improvement, and i look forward to seeing those changes as well. Could you ping me or this issue if you have a new release?

@marioortizmanero
Copy link
Author

that sounds like a great improvement, and i look forward to seeing those changes as well. Could you ping me or this issue if you have a new release?

Sure, I'll leave a comment here when we release the new version. By the way, you might be interested in https://dependabot.com/, it's useful to get notifications for new updates.

@robinvd
Copy link
Contributor

robinvd commented Nov 16, 2020

we already use dependabot for PR's like this #699

@stale
Copy link

stale bot commented Feb 14, 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 Feb 14, 2021
@stale stale bot closed this as completed Feb 24, 2021
@marioortizmanero
Copy link
Author

@robinvd Heads up: the new release is finally out! If you need help please let us know at ramsayleung/rspotify#218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement A new feature that would improve Spotifyd refactor Code changes that do not effect the functionality wontfix Issues that will not be fixed under any circumstances
Projects
None yet
Development

No branches or pull requests

3 participants