-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Italy state seed generation #3722
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seand7565
force-pushed
the
fix_italy_states
branch
from
August 4, 2020 17:47
25cff3e
to
aa3b311
Compare
spaghetticode
approved these changes
Aug 4, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seand7565 LGTM 👍 thanks!
kennyadsl
reviewed
Sep 4, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple of comments, thanks @seand7565
seand7565
force-pushed
the
fix_italy_states
branch
2 times, most recently
from
October 1, 2020 13:29
ebbb356
to
c9c60d8
Compare
Previously, Italy was using the top-level `subregions` - `regions` in Carmen to generate states, however for a mailing address this is incorrect, and it instead needs to use `provinces`, which is nested as the second level of `subregions` in Carmen. This fixes the seed generation, allowing for more countries to make this switch as necessary (there are a handful of other countries that need this) If you want an easy (and possibly destructive) method to update your states, see the following gist, which contains a rake task for you to run: https://gist.github.com/seand7565/6342d7ce7a89b2412fcb4eab7f2c8a61
seand7565
force-pushed
the
fix_italy_states
branch
from
October 1, 2020 13:30
c9c60d8
to
b343b96
Compare
jarednorman
approved these changes
Oct 2, 2020
@kennyadsl if this good to go for you? Feel free to merge if yes! |
kennyadsl
approved these changes
Oct 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Previously, Italy was using the top-level
subregions
-regions
in Carmen to generate states, however for a mailing address this
is incorrect, and it instead needs to use
provinces
, which isnested as the second level of
subregions
in Carmen. This fixes theseed generation, allowing for more countries to make this switch
as necessary (there are a handful of other countries that need this)
Also, adds aThis has been moved to a rake task, if you wantstate:regenerate
rake task that updates states forcountries that were generated incorrectly, like Italy. This is so
people updgrading from v2.10 to v2.11 can easily update to the
correct state list.
this functionality, you'll need to copy it over into your own code!
https://gist.github.com/seand7565/6342d7ce7a89b2412fcb4eab7f2c8a61
This PR only fixes Italy, but lays down some structure to allow fixing
other countries that need this treatment. For instance, if Spain needs
to use the nested subregions instead of the top-level subregions, we
just need to add Spains ISO code to
countries_that_use_nested_subregions
in
states.rb
and the ISO array on line 7 ofstates.rake
.Ref: #3670
Checklist: