-
-
Notifications
You must be signed in to change notification settings - Fork 929
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(location): Pad en_US ZIP codes left to 5 characters if needed #2278
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## next #2278 +/- ##
==========================================
- Coverage 99.60% 99.59% -0.01%
==========================================
Files 2644 2644
Lines 245770 245617 -153
Branches 1085 1079 -6
==========================================
- Hits 244798 244630 -168
- Misses 945 960 +15
Partials 27 27
|
Team decision This looks like a temporary workaround that is tailored for en-us only, but doesn't consider other zip code formats such as Poland's: |
It would be nice if ranges in addition to patterns could be specified. Realistically, most states don't seem to utilize the full range. |
There's already support for patterns, but it seems to be exclusive to the usage of zip code by state. Could they be merged? |
in many countries, postcodes/ZIP codes don't match up with state boundaries. For example note in that Wikipedia article "Postal districts and their subdivisions are not related to the administrative division of Poland." So I doubt there will be much usage of this outside of en_US. |
Actually, it looks like the proposed PR could be useful for Mexico, specifically the mexico City boroughs. The Mexican zip codes are modelled after US zip codes, with some mexico city zip codes prefixed with zeroes. |
I tried implementing it for the Netherlands and our provinces, but I ran into a problem as our postal codes also contain letters. Dutch postal codes are in the format I found the numeric ranges for the provinces, and for the letters we could just use 2 random letters. |
My feeling is while we should support this for legacy reasons for en_US we shouldn't put too much effort into trying to make this work in every locale- if this feature didn't currently exist I suspect it would be rejected as a feature request, since we generally avoid features that allow you to create consistent data between methods (eg city that matches the state). |
I saw from the meeting notes you want to try to achieve this using patterns instead so it would work for more countries, but not sure how the current behavior could be replicated using string patterns as it includes numeric ranges. |
|
have changed this to use string patterns passed to |
Oh 😲 that is very interesting |
There seems to be a merge conflict. |
fix #2277
Added a test and a fix. Not sure if this is the best long-term solution, but since only en_US uses this method currently, it seems a fairly low-impact solution.
Also fixed Puerto Rico range so its not always 00000