jmtellez / CLI-mate

Node CLI app that gives you the weather forecast for a given city.
6 stars 10 forks source link

Geolocation #3

Closed jmtellez closed 3 years ago

jmtellez commented 3 years ago

When no command is passed, cli-mate should return weather data from where the user is located.

aaronzshey commented 3 years ago

If you're using a bash script, you could try something like:

ipaddr=curl ipecho.net/plain ; echo
curl https://freegeoip.app/xml/$ipaddr

I can help!

In fact, you don't even need to get the ip address - curling the bare freegeoip address will default to the host iP.

jmtellez commented 3 years ago

you can make the request with node. LMK and I can assign it to you.

aaronzshey commented 3 years ago

yes, we can use node-fetch instead to gather geolocation information - freegeoip.app returns longitude and latitude. If you can give me a pull request, I can do that right now. For now, I'll just fork it.

jmtellez commented 3 years ago

Use postman-request

aaronzshey commented 3 years ago

Thanks. Please send a pull request when you can.

jmtellez commented 3 years ago

Fork the project and then open PR when you are done!

aaronzshey commented 3 years ago

@jmtellez How do I link the fork to a PR? 😅 Sorry, I've never worked with pull requests before.

jmtellez commented 3 years ago

Push your changes to your forked repo, then head over to the original repo and click on Pull request tab and create a PR. Select the branch you are wanting to merge your code to and add me as a reviewer.

Here is a detailed doc on how to do it: https://www.digitalocean.com/community/tutorials/how-to-create-a-pull-request-on-github

aaronzshey commented 3 years ago

Thanks! I noticed this in the code:

switch (location) {
  case undefined:

How would we differentiate between showing the menu when the location is undefined, and automatically getting the IP address?

jmtellez commented 3 years ago

What you can do is create a function like utils/geocode.js and instead of hitting map box endpoint hit freegeoip endpoint. We can call that function and still get lat and Lon then call forecast(). No need to show the menu once this is implemented

aaronzshey commented 3 years ago

freegeoip only returns city - I'll see if I can get the city out of freegeoip, and then hopefully pass that back into the other endpoints.

jmtellez commented 3 years ago

You can pass city to forecast() it should work

aaronzshey commented 3 years ago

@jmtellez the function works for now - I'll try to pass the city in tomorrow.

8

arpitvaghela commented 3 years ago

location can be an optional argument as we are implementing this

aaronzshey commented 3 years ago

@arpitvaghela that's handled with the switch statements. @jmtellez Did you see #8 ?

arpitvaghela commented 3 years ago

I am trying to solve the #11, will change my implementation to support this as well

cosmoimai commented 3 years ago

Sir we just have to use geolocation for that. Is it?

cosmoimai commented 3 years ago

sir, we have to provide your own token? because it is not working means that process.env.MAPBOX is not working in geocode line 7

aaronzshey commented 3 years ago

@cosmoimai did you generate your own api keys? Also @arpitvaghela how should we implement the automatic geolocation with the unit flags?

cosmoimai commented 3 years ago

@CarlyRaeJepsenStan Sir I think we have to generate our own api key or some sort of token i think Through some request

aaronzshey commented 3 years ago

@cosmoimai what kind of errors are you receiving?

jmtellez commented 3 years ago

@cosmoimai you need to create both weatherstack and mapbox api keys and set their corresponding env vars.

cosmoimai commented 3 years ago

@CarlyRaeJepsenStan sir, actually it is giving unable to locate

aaronzshey commented 3 years ago

Using the autolocation:

/* 
== Autolocation - should be fired on case undefined

const autoLocate = require("./utils/autolocate")

autoLocate((err, city) => {
 //console.log(city) ;

case geocode, etc.
})

==
*/

Placed here for future reference.

aaronzshey commented 3 years ago

22