starred/rspotify

Fork 0

mirror of https://github.com/ramsayleung/rspotify.git synced 2026-04-26 07:55:55 +03:00

[GH-ISSUE #135] Consider making the cache file a feature #48

New issue

Closed

opened 2026-02-27 20:22:47 +03:00 by kerem · 6 comments

kerem commented

2026-02-27 20:22:47 +03:00

Owner

Originally created by @marioortizmanero on GitHub (Oct 10, 2020).
Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/135

The cache file feature is something I'm not sure how I feel about. While it's definitely useful, I'm not sure if it should be the default behaviour. It could be moved into a feature, which when activated, would use the cache file anywhere possible. Or instead of having do_something and do_something_without_cache, it could be do_something and do_something_with_cache.

What do you think? I need some opinions on this. I don't actually plan on changing anything but I want to propose any breaking changes before the next release.

Originally created by @marioortizmanero on GitHub (Oct 10, 2020). Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/135 The cache file feature is something I'm not sure how I feel about. While it's definitely useful, I'm not sure if it should be the default behaviour. It could be moved into a feature, which when activated, would use the cache file anywhere possible. Or instead of having `do_something` and `do_something_without_cache`, it could be `do_something` and `do_something_with_cache`. What do you think? I need some opinions on this. I don't actually plan on changing anything but I want to propose any breaking changes before the next release.

kerem

2026-02-27 20:22:47 +03:00

closed this issue
added the
help wanted

discussion
labels

kerem commented

2026-02-27 20:22:48 +03:00

Author

Owner

@ramsayleung commented on GitHub (Oct 11, 2020):

Yes, I think it's more proper to have a switch to control how does it work. My original thought was using cache_path as a trigger, as what I commented in #129:

    #[builder(default = "PathBuf::from(\".spotify_token_cache.json\")")]
    pub cache_path: PathBuf,
	// I am thinking about whether it's better to make cache_path optional
    // so we can decide to whether save the token into file or not by checking 
    // if cache_path exists, so we can remove function get_token_without_cache

Now, probably it's more suitable to make it as a feature, switching on compile time instead of runtime, as what you suggests.

@ramsayleung commented on GitHub (Oct 11, 2020): Yes, I think it's more proper to have a switch to control how does it work. My original thought was using `cache_path` as a trigger, as what I commented in #129: ```rust #[builder(default = "PathBuf::from(\".spotify_token_cache.json\")")] pub cache_path: PathBuf, // I am thinking about whether it's better to make cache_path optional // so we can decide to whether save the token into file or not by checking // if cache_path exists, so we can remove function get_token_without_cache ``` Now, probably it's more suitable to make it as a feature, switching on compile time instead of runtime, as what you suggests.

kerem commented

2026-02-27 20:22:48 +03:00

Author

Owner

@ramsayleung commented on GitHub (Oct 24, 2020):

Any other proposals? I am gonna to start to work on this feature on this weekend as what @marioortizmanero suggests, makes cache file as a new feature.

@ramsayleung commented on GitHub (Oct 24, 2020): Any other proposals? I am gonna to start to work on this feature on this weekend as what @marioortizmanero suggests, makes cache file as a new feature.

kerem commented

2026-02-27 20:22:48 +03:00

Author

Owner

@marioortizmanero commented on GitHub (Oct 24, 2020):

Can you cross off more things from the meta issue before this? I would wait a bit more before this, let's see if anyone opposes the idea.

@marioortizmanero commented on GitHub (Oct 24, 2020): Can you cross off more things from the meta issue before this? I would wait a bit more before this, let's see if anyone opposes the idea.

kerem commented

2026-02-27 20:22:48 +03:00

Author

Owner

@ramsayleung commented on GitHub (Oct 24, 2020):

Sure! We could leave this issue alone for a while.

@ramsayleung commented on GitHub (Oct 24, 2020): Sure! We could leave this issue alone for a while.

kerem commented

2026-02-27 20:22:48 +03:00

Author

Owner

@marioortizmanero commented on GitHub (Dec 28, 2020):

This has been inactive for a while and noone has left their opinion so I would say we can introduce the cache-file feature. Should it be activated by default?

Also, do you still want to work on it @ramsayleung?

@marioortizmanero commented on GitHub (Dec 28, 2020): This has been inactive for a while and noone has left their opinion so I would say we can introduce the `cache-file` feature. Should it be activated by default? Also, do you still want to work on it @ramsayleung?

kerem commented

2026-02-27 20:22:48 +03:00

Author

Owner

@ramsayleung commented on GitHub (Dec 30, 2020):

Should it be activated by default?

I think it would be good to active this feature by default, just makes rspotify work as before.

Also, do you still want to work on it @ramsayleung?

Not yet, I am working on this issue: https://github.com/ramsayleung/rspotify/issues/163, I would like to work on this feature after merging the PR of #163, and we could say we have crossed off most of TODO items in #127

@ramsayleung commented on GitHub (Dec 30, 2020): > Should it be activated by default? I think it would be good to active this feature by default, just makes `rspotify` work as before. > Also, do you still want to work on it @ramsayleung? Not yet, I am working on this issue: https://github.com/ramsayleung/rspotify/issues/163, I would like to work on this feature after merging the PR of #163, and we could say we have crossed off most of TODO items in #127

kerem referenced this issue

2026-02-27 20:23:47 +03:00

[PR #48] [MERGED] Remove an unnecessary mut from current_user_saved_tracks_delete #208