-
Notifications
You must be signed in to change notification settings - Fork 13
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
Split OWM API or make it more explicit about valid URLs #64
Comments
I think this is not to have to release a new version if the urls change but I do agree. One way to solve this would be to have an implicit holding the default urls. I do like the idea overall but we have to consider a couple of things:
cc @chuwy |
IMHO easier because after the split
Probably more confusing, that's true, the above point still stands though. I've probably overcomplicated this. We don't need "history" client, "current/forecast" client and "both" client, we just need "current/forecast" client and "both" client, because:
|
that's something we'd need to differentiate on but I agree on the other points 👍 |
Few points from me:
|
Part of this issue will be covered by #71 |
As far as I know it's not currently possible to have the whole API working at the same time - when the passed url is
history.openweathermap.org
, only methodshistory*
work. When passedpro.openweathermap.org
orapi.openweathermap.org
, onlyforecast*
andcurrent*
methods succeed. The concept of passing custom urls also does not seem useful to me, as there are only 3 choices (correct me if I'm wrong here).My proposition:
Currently
Client
is a trait that has one abstract methodreceive
. The rest are concrete methods callingreceive
. We can split them and hardcode API urls inside traits, then use them traits as mixins. Quick simplified proof of concept:The text was updated successfully, but these errors were encountered: