Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

improve location descriptions #69

Merged
merged 7 commits into from May 29, 2018
Merged

improve location descriptions #69

merged 7 commits into from May 29, 2018

Conversation

niklaslong
Copy link
Contributor

closes #58

@niklaslong niklaslong added this to To do in 0.1.0 via automation May 28, 2018
@niklaslong niklaslong moved this from To do to In dev in 0.1.0 May 28, 2018
@niklaslong niklaslong changed the title [WIP] improve location descriptions improve location descriptions May 29, 2018
@niklaslong
Copy link
Contributor Author

@marcoow this is ready for review

@niklaslong niklaslong requested a review from marcoow May 29, 2018 09:30
@niklaslong niklaslong moved this from In dev to In review in 0.1.0 May 29, 2018
Copy link
Member

@marcoow marcoow left a comment

Choose a reason for hiding this comment

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

I'm not sure I'm not seeing it but I think this is missing defaulting the label to what we currently use in the view.

OpenAQ.Locations.get_locations(lat, lon)
locations = OpenAQ.Locations.get_locations(lat, lon)

locations
Copy link
Member

Choose a reason for hiding this comment

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

it should be sufficient to do this only for the locations that don't have labels yet as that is very unlikely to change

@niklaslong
Copy link
Contributor Author

niklaslong commented May 29, 2018

Ah right - I set the label to the same value as the identifier initially - then update it.
I suppose we could also set the label to null and use the identifier if it's null in the view?
What solution do you prefer?

@marcoow
Copy link
Member

marcoow commented May 29, 2018

I'd do the defaulting in the view as otherwise you cannot really find out which one has been updated already and which one hasn't.

@marcoow marcoow merged commit 845c0c7 into mainmatter:master May 29, 2018
0.1.0 automation moved this from In review to done May 29, 2018
@marcoow marcoow deleted the geocoding branch May 29, 2018 10:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
0.1.0
  
done
Development

Successfully merging this pull request may close these issues.

Improve location description
2 participants