ladybug-tools / ladybug-legacy

:beetle: Ladybug is an environmental plugin for Grasshopper.
http://ladybug.tools
Other
192 stars 82 forks source link

Edited Location Finder Component #451

Closed apoorvgoyal closed 5 years ago

apoorvgoyal commented 5 years ago

Google changed the API to require API Key. Added option to input users own custom API Key.

chriswmackey commented 5 years ago

Thanks for the pull request, @apoorvgoyal . That's a shame that Google makes you pay to access their maps API now but it's great that you are updating the component.

One other thing that I noticed other than an updated date is that there's no error checking for when nothing is connected to the component. If you can add the date and put n some code in to stop it from producing meaningless warnings when nothing is connected, I will merge it in. image

image

chriswmackey commented 5 years ago

@apoorvgoyal , Let me know via a comment on this PR when it's ready to reviewed and merged. And thank you, again, for making the changes

apoorvgoyal commented 5 years ago

Done.

Apoorv Goyal LEED® AP BD+C, WELL® AP, AIA-Intl. Associate

*Senior Sustainable Design Specialist - *HOK Inc. (New York)

Master in Design Studies (Energy and Environments) - Harvard Graduate School of Design

Bachelors in Architecture - School of Planning and Architecture, Delhi, India

On Sun, 21 Oct 2018 at 13:19, Chris Mackey notifications@github.com wrote:

Thanks for the pull request, @apoorvgoyal https://github.com/apoorvgoyal . That's a shame that Google makes you pay to access their maps API now and it's great that you are updating it.

One other thing that I noticed other than an updated date is that there's no error checking for when nothing is connected to the component. If you can add the date and put n some code in to stop it from producing meaningless warnings when nothing is connected, I will merge it in. [image: image] https://user-images.githubusercontent.com/5567574/47270033-e2356a80-d533-11e8-8edf-2d5a19458ce3.png

[image: image] https://user-images.githubusercontent.com/5567574/47270039-eb263c00-d533-11e8-9e5f-9bb1784e45ed.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mostaphaRoudsari/ladybug/pull/451#issuecomment-431686790, or mute the thread https://github.com/notifications/unsubscribe-auth/AWX5JGK05x_ajYxm_SKyTtme4k6DqVqEks5unKyegaJpZM4XefnI .

apoorvgoyal commented 5 years ago

Well, to make it up for it, they give you $300 monthly credit. So technically its still free, unless it goes to like thousands of queries per second.

Apoorv Goyal LEED® AP BD+C, WELL® AP, AIA-Intl. Associate

*Senior Sustainable Design Specialist - *HOK Inc. (New York)

Master in Design Studies (Energy and Environments) - Harvard Graduate School of Design

Bachelors in Architecture - School of Planning and Architecture, Delhi, India

On Mon, 22 Oct 2018 at 09:50, Apoorv Goyal apoorvgoyal@gmail.com wrote:

Done.

Apoorv Goyal LEED® AP BD+C, WELL® AP, AIA-Intl. Associate

*Senior Sustainable Design Specialist - *HOK Inc. (New York)

Master in Design Studies (Energy and Environments) - Harvard Graduate School of Design

Bachelors in Architecture - School of Planning and Architecture, Delhi, India

On Sun, 21 Oct 2018 at 13:19, Chris Mackey notifications@github.com wrote:

Thanks for the pull request, @apoorvgoyal https://github.com/apoorvgoyal . That's a shame that Google makes you pay to access their maps API now and it's great that you are updating it.

One other thing that I noticed other than an updated date is that there's no error checking for when nothing is connected to the component. If you can add the date and put n some code in to stop it from producing meaningless warnings when nothing is connected, I will merge it in. [image: image] https://user-images.githubusercontent.com/5567574/47270033-e2356a80-d533-11e8-8edf-2d5a19458ce3.png

[image: image] https://user-images.githubusercontent.com/5567574/47270039-eb263c00-d533-11e8-9e5f-9bb1784e45ed.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mostaphaRoudsari/ladybug/pull/451#issuecomment-431686790, or mute the thread https://github.com/notifications/unsubscribe-auth/AWX5JGK05x_ajYxm_SKyTtme4k6DqVqEks5unKyegaJpZM4XefnI .

apoorvgoyal commented 5 years ago

Chris its ready. You should see the changes in the pull request

On Mon, Oct 22, 2018, 12:26 PM Chris Mackey notifications@github.com wrote:

@apoorvgoyal https://github.com/apoorvgoyal , Let me know via a comment on this PR when it's ready to reviewed and merged. And thank you, again, for making the changes

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mostaphaRoudsari/ladybug/pull/451#issuecomment-431886782, or mute the thread https://github.com/notifications/unsubscribe-auth/AWX5JLokDccALRXnwx5lt3F6C4CwEQVPks5unfG5gaJpZM4XefnI .

chriswmackey commented 5 years ago

Looks great @apoorvgoyal . Thanks for taking care of this. Merged!