spotify-tui icon indicating copy to clipboard operation
spotify-tui copied to clipboard

Future breaking update of Rspotify

Open marioortizmanero opened this issue 3 years ago • 3 comments

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) for help in some discussions relating the future of Rspotify:

  • https://github.com/ramsayleung/rspotify/issues/134 Optional parameters
  • https://github.com/ramsayleung/rspotify/issues/135 Consider making the cache file a feature or not default
  • https://github.com/ramsayleung/rspotify/issues/137 Clean up and re-structure the errors
  • CHANGELOG.md What we've already changed so far
  • https://github.com/ramsayleung/rspotify/issues/127 What we are working on

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.

marioortizmanero avatar Oct 18 '20 18:10 marioortizmanero

Looking forward to these updates @marioortizmanero @ramsayleung. Going to rely on the power of Rust to guide me in fixing the breaking changes.

Rigellute avatar Feb 27 '21 13:02 Rigellute

I'm very happy with the progress we're making so far! There's still quite a lot to do, but compared to the state the library was last year, so much has been improved already. Meanwhile you can provide us with some opinions on issues we still are discussing how to solve. I'll try to make them as concise as possible because I know the threads are lengthy now. Answer whatever you can/want, I'll send this to more devs.

  • https://github.com/ramsayleung/rspotify/issues/4 We want to make it possible to have automatically refreshing tokens (not sure how you handle token refreshing right now) and cached tokens. Would you rather have features in the Cargo.toml to configure what type of token you want to use (token-refreshing and token-cached, for example), or runtime configuration (client.set_cached() and client.set_refreshing() for example)?

    The latter would make it possible to use different combinations of tokens in the same program (i.e. a refreshing but not cached one in a part of the program, and a cached but not refreshing in another part), but I'm not sure if that's actually needed. Also unsure if there are more elegant ways of configuring that.

  • https://github.com/ramsayleung/rspotify/issues/134 How would you rather use optional parameters:

    • The conventional and simplest way, using Option: fn endpoint(&self, market: Option<Market>), using it with endpoint(None) or endpoint(Some(Market::FromToken)).
    • A signature like fn endpoint<M: Into<Option<Market>>(&self, market: M) so that it can be used as endpoint(Market::FromToken), without Some. The downside is that generics will generate 2^N copies of that function, where N is the number of combinations for the optional parameters used (many times that'll be 1 anyway).
    • A more complex solution (see this blogpost).
  • https://github.com/ramsayleung/rspotify/pull/166 We want to add an easier interface to iterate methods with paginated results. Should it be an optional feature? In case the feature is activated, should it replace the original methods, or be added as a new endpoint with the _stream suffix? Should it be enabled by default?

These are decisions that are quite complicated to take without insight from others, so we'd appreciate any possible help :)

marioortizmanero avatar Feb 27 '21 14:02 marioortizmanero

Heads up: the new release is finally out! If you need help please let us know at https://github.com/ramsayleung/rspotify/issues/218

marioortizmanero avatar Oct 18 '21 23:10 marioortizmanero