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 #624

Open
marioortizmanero opened this issue Oct 18, 2020 · 3 comments
Open

Future breaking update of Rspotify #624

marioortizmanero opened this issue Oct 18, 2020 · 3 comments

Comments

@marioortizmanero
Copy link

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:

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.

@Rigellute
Copy link
Owner

Rigellute commented Feb 27, 2021

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

@marioortizmanero
Copy link
Author

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.

  • Automatic Re-authentication ramsayleung/rspotify#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.

  • Optional parameters ramsayleung/rspotify#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).
  • [WIP/Proposal] Add Stream/Iterator based versions of methods with paginated results ramsayleung/rspotify#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
Copy link
Author

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
None yet
Projects
None yet
Development

No branches or pull requests

2 participants