search

InputUsername / listenbrainz-rs

ListenBrainz API bindings for Rust
https://crates.io/crates/listenbrainz
MIT License
11 stars 6 forks source link

Look into setting alternative Listenbrainz hosts for a session #2

Closed shymega closed 3 years ago

shymega commented 3 years ago

As the title says...

InputUsername commented 3 years ago

I think we can add an API URL field to raw::client::Client and create a Client::with_url (or similar) constructor function to configure it. Client::new can then use that and pass the default API URL.

shymega commented 3 years ago

Got it working... I think. Pushing my branch for evaluation - do you want me to open a PR? Need to adjust unit tests, and add additional examples. Other than that, looks like a clean fix ...

shymega commented 3 years ago

OK, I can confirm the fix works examples - it works either if you use Client::new() or Client::new_with_url(url: &str). I'm pleased with the fix. The only things that need fixing up now are: unit tests and examples - we should include tests for both unofficial Listenbrainz hosts (if any exist?), and official hosts. The same goes with examples.

shymega commented 3 years ago

Having said that - we don't seem to have unit tests?

InputUsername commented 3 years ago

Perfect! Yeah, you can open a PR.

InputUsername commented 3 years ago

Having said that - we don't seem to have unit tests?

Correct, there are currently no unit tests. Definitely something that needs to be added. I've made an issue (#5)

shymega commented 3 years ago

Pushed to the feature/alternative-hosts branch. Opening PR now...

shymega commented 3 years ago

Having said that - we don't seem to have unit tests?

Correct, there are currently no unit tests. Definitely something that needs to be added. I've made an issue (#5)

Noted. Looking at issue #5 now.