Skip to content

feat(location): update en county list#2238

Merged
ST-DDT merged 5 commits into
faker-js:nextfrom
matthewmayer:en-county
Sep 1, 2023
Merged

feat(location): update en county list#2238
ST-DDT merged 5 commits into
faker-js:nextfrom
matthewmayer:en-county

Conversation

@matthewmayer

@matthewmayer matthewmayer commented Jul 5, 2023

Copy link
Copy Markdown
Contributor

re #2207 #1944

Updates the en county list to be a lomger list of US-esque county names instead of a
truncated list of GB county names.
@matthewmayer matthewmayer requested a review from a team as a code owner July 5, 2023 08:05
@matthewmayer matthewmayer changed the title locale(location): update en county list to match en_US feat(location): update en county list to match en_US Jul 5, 2023
@matthewmayer matthewmayer self-assigned this Jul 5, 2023
@matthewmayer matthewmayer added p: 1-normal Nothing urgent c: locale Permutes locale definitions m: location Something is referring to the location module labels Jul 5, 2023
@codecov

codecov Bot commented Jul 5, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #2238 (3a0fc17) into next (8e5bc22) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             next    #2238      +/-   ##
==========================================
- Coverage   99.60%   99.60%   -0.01%     
==========================================
  Files        2760     2760              
  Lines      251164   251266     +102     
  Branches     1081     1083       +2     
==========================================
+ Hits       250176   250274      +98     
- Misses        961      965       +4     
  Partials       27       27
Files Changed Coverage Δ
src/locales/en/location/county.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes
ejcheng
ejcheng previously approved these changes Jul 8, 2023
View reviewed changes
@ST-DDT

ST-DDT commented Jul 9, 2023

Copy link
Copy Markdown
Member

Shouldn't we mix other states counties in as well otherwise en == en_US.
Maybe lets discuss this in the next team meeting.
@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Jul 9, 2023
@matthewmayer

matthewmayer commented Jul 9, 2023

Copy link
Copy Markdown
Contributor Author

We don't do that for state()
@ST-DDT

ST-DDT commented Aug 4, 2023

Copy link
Copy Markdown
Member

There seems to be merge conflicts. Could you please fix them?
@ejcheng ejcheng added the needs rebase There is a merge conflict label Aug 4, 2023
@ejcheng ejcheng removed the needs rebase There is a merge conflict label Aug 5, 2023
@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision labels Aug 17, 2023
@ST-DDT

ST-DDT commented Aug 17, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Team decision

We will use both the "British" and US county names for en.
@matthewmayer

matthewmayer commented Aug 18, 2023

Copy link
Copy Markdown
Contributor Author

Done.

Note this is a good example of why you don't always want to sort definitions alphabetically re #2265
ST-DDT
ST-DDT reviewed Aug 18, 2023
View reviewed changes
Comment thread src/locales/en/location/county.ts
@ST-DDT ST-DDT changed the title feat(location): update en county list to match en_US feat(location): update en county list Aug 20, 2023
@ST-DDT ST-DDT enabled auto-merge (squash) September 1, 2023 18:30
@ST-DDT ST-DDT merged commit 6bb4775 into faker-js:next Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: locale Permutes locale definitions m: location Something is referring to the location module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug

5 participants

@matthewmayer @ST-DDT @Shinigami92 @ejcheng @xDivisionByZerox