[Enhancement] Add API Trait

[kevinmatthes]

This is an initial suggestion based on the discussion in #86. I will add some explanations on the changes in a review. To show the basic ideas of the trait based approach, I refactored the location determination using the newly introduced API trait as an example.

[kevinmatthes]

Here are some comments on the changes.

[kevinmatthes]

I am looking forward to your feedback, @tobealive!

[ttytm]

Thanks a lot for this beautiful work! I glanced over it and I'll happily dive deeper into it this night :)

[kevinmatthes]

I have found some approaches to unite the querying functions for the location; I still need to test them but it looks already quite good. I will upload the changes then.

[ttytm]

Thanks again for the good work @kevinmatthes :)

For the location, I would use Open Street Map as the primary source for any weather api. It provides the best details for displaying the address and by default we would have a consistency in the address. Also, not all weather APIs provide geocoding.

The main part is the implementation for the weather data. I haven't quite thought through what the implementation will look like in the end, I have to admit.

I think it would be great to get far enough with this extension to use it directly as a basis for implementing another weather provider like openweathermap in a seperate PR based on this enhancement.

What's your thoughts on that?

[github-actions]

Test Results

  3 files  ±0    3 suites  ±0   3s ⏱️ ±0s
  9 tests ±0    9 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
27 runs  ±0  27 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 84440b8. ± Comparison against base commit cf2234b.

♻️ This comment has been updated with latest results.

[kevinmatthes]

I will try to finalise the tested approach an upload it. I will also mark this PR ready for review, then. If you agree with all changes I made, I would then begin to also refactor the weather API using the trait approach after this PR is merged.

For a new weather API, please provide me the required information such as the querying URL and which information to format into it. I assume that a separate issue would be the best place for these information such that other users can add their ideas on the new weather API, as well.

[kevinmatthes]

These are the changes I tested yesterday. I tried to unify the two location provision service queries but I always received compiler complaints regarding lifetime issues. Thus, I removed all experimental features to ensure the current state to be working while the unification is still possible and now even easier. Maybe you have got an idea, @tobealive?

Before merging this PR, please counter-check that the querying URLs are all unchanged. Although I checked this three times myself and never found any mistakes, typos in the querying URLs are the most critical point regarding this PR as they would break application.

[ttytm]

It's a packed week. Sorry for taking that long. I didn't had a chance to walk the dog. I'll take that poor good boy out and try to come back to this later, else you'll here from me tomorrow on this. Thanks for working on this!

[ttytm]

lvgtm!

It'll be merged tomorrow(which is more today, but after sleep) 👍

[ttytm]

For a new weather API, please provide me the required information such as the querying URL and which information to format into it. I assume that a separate issue would be the best place for these information such that other users can add their ideas on the new weather API, as well.

Yep, I'll get an overview of another api like openweathermap and make a list in separate issue.

Also the added functional in this PR is to classify as semver minor, right? So we'll increase version before merging coming changes 👍.

[kevinmatthes]

I would rather make that a patch release. This crate is not primarily a library and the changes are only internal.

[ttytm]

Thanks what we'll do 🙂👍. Thanks!