search

felixhageloh / weather-widget

A weather widget for Übersicht
64 stars 32 forks source link

To Promises, fallback Location API, code reorganisation #51

Closed matthewfallshaw closed 5 years ago

matthewfallshaw commented 5 years ago

A handful of changes:

  1. Move API configuration into a config object for each API service ((nearly) all of the logic relating to each service in one place (I didn't find an elegant way to avoid concatenating the lat/long onto the URL for the weather service)).
  2. Add a fallback Location service (I'm not sure if the static location option is working for you; it wasn't working for me without lat/long, so I've added a service to look those up from the city/region).
  3. Use Promises (which I find much easier to follow that callback hell).

Then a handful more that you're much more likely to decide not to include:

  1. Prefer using the keychain over hardcoding plaintext secrets into code ($(security find-generic-password -a ubersicht -s ipstack -w) pulls a secret from the keychain; see https://www.netmeister.org/blog/keychain-passwords.html for more context and how to put the secret into your keychain in the first place).
  2. Replace ipstack.com with ip-api.com (which doesn't require an account, and which gives me much more accurate geolocation).

Then some stuff you totally should not include, my tweaks to local my config, but that I am selfishly not going to remove since it's in my master and it would be extra work.

felixhageloh commented 5 years ago

Hi thanks for the patch! I am not able to maintain this widget anymore (as you might have noticed) and I am thinking of archiving this repo. If you would like to fork your own widget and submit it to the gallery, I would be happy to add it!

matthewfallshaw commented 5 years ago

Thanks for responding. I don't think I can take on that commitment either, so I'll close the pull request.