-
Notifications
You must be signed in to change notification settings - Fork 223
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
Deduplicate code and fix a few clippy complaints #549
Deduplicate code and fix a few clippy complaints #549
Conversation
Awesome! Thank you for your contribution - I'll have a look now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I'll merge it in!
self.send_empty_json(request)?; | ||
Ok(()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah thank you for taking care of these, I thought they might be redundant
// From https://github.com/KRTirtho/spotube/blob/9b024120601c0d381edeab4460cb22f87149d0f8/lib%2Fservices%2Fcustom_spotify_endpoints%2Fspotify_endpoints.dart keep and eye on this and change accordingly | ||
const EXTENSIONS_JSON: &str = r#"{ | ||
"persistedQuery": { | ||
"version": 1, | ||
"sha256Hash": "eb3fba2d388cf4fc4d696b1757a58584e9538a3b515ea742e9cc9465807340be" | ||
} | ||
}"#; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if there's a better place to put this, but here is fine for now.
// 0JQ5DAUnp4wcj0bCb3wh3S -> Daily mixes | ||
let json_query = self.build_home_request("spotify:section:0JQ5DAUnp4wcj0bCb3wh3S"); | ||
pub fn get_section(&self, section_uri: &str) -> Result<MixedView, Error> { | ||
let request = self | ||
.get("pathfinder/v1/query", Some("api-partner.spotify.com"))? | ||
.query("operationName", "homeSection") | ||
.query("variables", &json_query.0.to_string()) | ||
.query("extensions", &json_query.1.to_string()); | ||
.query("variables", &self.build_home_request(section_uri)?) | ||
.query("extensions", EXTENSIONS_JSON); | ||
|
||
// Extract the playlists | ||
let result = self.load_and_return_home_section(request)?; | ||
self.load_and_return_home_section(request) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice abstraction for this.
While I was working on #548, I noticed a few areas where code was unnecessarily copied or did extra work/computation that it didn't need to do. Most of that was in the WebApi client struct, but there were a few other instances elsewhere as well. There were also some issues that clippy was complaining about which I took care of.
As far as I can tell, everything still works just as expected after these changes.