Add provinces to Australian labels #638

Merged
merged 2 commits into from Aug 19, 2016

Conversation

Projects
None yet
4 participants
@avulfson17
Contributor

avulfson17 commented Aug 17, 2016

Fixes #615

@avulfson17 avulfson17 self-assigned this Aug 17, 2016

helper/labelSchema.js
@@ -13,12 +13,17 @@ module.exports = {
'USA': {
'borough': getFirstProperty(['borough']),
'local': getFirstProperty(['locality', 'localadmin', 'county']),
- 'regional': getUsOrCaState,
+ 'regional': getUsOrCaOrAusState,

This comment has been minimized.

@orangejulius

orangejulius Aug 19, 2016

Member

after how many countries do we rename this to something like getRegionalValue? 3 feels like a good time :)

@orangejulius

orangejulius Aug 19, 2016

Member

after how many countries do we rename this to something like getRegionalValue? 3 feels like a good time :)

helper/labelSchema.js
'country': getUSACountryValue
},
+ 'AUS': {
+ 'local' : getFirstProperty(['locality', 'localadmin']),
+ 'regional' : getUsOrCaOrAusState,

This comment has been minimized.

@avulfson17

avulfson17 Aug 19, 2016

Contributor

is there a reason that this function isn't called here? why does it just store the variable name?

@avulfson17

avulfson17 Aug 19, 2016

Contributor

is there a reason that this function isn't called here? why does it just store the variable name?

This comment has been minimized.

@orangejulius

orangejulius Aug 19, 2016

Member

There is a reason! It's a bit confusing. The labelGenerator.js module which actually generates the labels loads this module, and picks the correct country's schema to use, and then calls the function from that schema

@orangejulius

orangejulius Aug 19, 2016

Member

There is a reason! It's a bit confusing. The labelGenerator.js module which actually generates the labels loads this module, and picks the correct country's schema to use, and then calls the function from that schema

@orangejulius

This comment has been minimized.

Show comment
Hide comment
@orangejulius

orangejulius Aug 19, 2016

Member

LGTM now!

Member

orangejulius commented Aug 19, 2016

LGTM now!

@trescube

This comment has been minimized.

Show comment
Hide comment
@trescube

trescube Aug 19, 2016

Contributor

:shipit:

Contributor

trescube commented Aug 19, 2016

:shipit:

@orangejulius orangejulius merged commit 6f09259 into master Aug 19, 2016

3 checks passed

approvals/lgtm this commit looks good
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dianashk dianashk removed the in progress label Aug 19, 2016

@orangejulius orangejulius removed the in review label Aug 19, 2016

@orangejulius orangejulius deleted the expand-aussie-labels branch Aug 19, 2016

je-l pushed a commit to nlsfi/pelias-api that referenced this pull request Aug 31, 2017

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