Skip to content

Rake geocode#10

Merged
sandraor merged 4 commits intosandraor:masterfrom
solsberg:rake_geocode
Jan 14, 2015
Merged

Rake geocode#10
sandraor merged 4 commits intosandraor:masterfrom
solsberg:rake_geocode

Conversation

@solsberg
Copy link

This is a new pull request to replace my previous one

The geocode rake task will now simply calculate lng/lat fields for the group records in meetups.yml where no such values already exist. It will use an 'address' field if it find one (this allows specifying a more accurate street address to geocode), otherwise it will use the location field to geocode to a general coordinate in the city.

I have also included a commit which allows the map to go ahead and draw even in the case when the user does not permit the geolocation API to lookup the current location. In this case the map is displayed to the extent of all the markers. Previously in this case the map would display without any of the markers.

Rakefile Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better approach than this? Perhaps a group API?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea, and I guess it could also provide a place in the code for data preparation using the meetups.com API that has been discussed for the future. I will attempt a refactoring later - hopefully this evening

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Let me know how it goes :D

@wifelette
Copy link

Adding explicit reference: emberjs#1904. It feels silly that this is needed... but it's useful to be able to see the status on the main PR page 😛

@wycats
Copy link

wycats commented Jan 13, 2015

@solsberg This is good stuff! I added a bunch of comments, but overall this looks sound.

@solsberg
Copy link
Author

I have reverted my commit whose functionality was subsumed by #9 and also taken up the @wycats suggestion of moving the core of the geocoding functionality into a MeetupsData module

sandraor added a commit that referenced this pull request Jan 14, 2015
@sandraor sandraor merged commit 6eafac1 into sandraor:master Jan 14, 2015
@sandraor
Copy link
Owner

Merged. Thanks, @solsberg!

@sandraor
Copy link
Owner

I just noticed that the map markers don't show up and the location links don't work when location detection is off in the browser. Is this related to solsberg@6626de2?

@alexclarkofficial
Copy link

My PR(#9) will fix this. Just working on getting the Retina images in now -
should be in within an hour.

On Tue, Jan 13, 2015 at 10:18 PM, Sandra notifications@github.com wrote:

I just noticed that the map markers don't show up and the location links
don't work when location detection is off in the browser. Is this related
to solsberg/website@6626de2
solsberg@6626de2
?


Reply to this email directly or view it on GitHub
#10 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants