Skip to content

refactor(locale)!: use snake case for all locale data#2910

Merged
ST-DDT merged 5 commits intonextfrom
refactor/locale/snake-case
May 25, 2024
Merged

refactor(locale)!: use snake case for all locale data#2910
ST-DDT merged 5 commits intonextfrom
refactor/locale/snake-case

Conversation

@ST-DDT
Copy link
Copy Markdown
Member

@ST-DDT ST-DDT commented May 18, 2024

Standardize file naming convention for the renaming locale files.

old replacement
faker.definitions.science.chemicalElement faker.definitions.science.chemical_element
faker.definitions.system.directoryPaths faker.definitions.system.directory_paths
faker.definitions.system.mimeTypes faker.definitions.system.mime_types

This is an attempt an fixing some TODOs in our sources.

@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs c: locale Permutes locale definitions breaking change Cannot be merged when next version is not a major release labels May 18, 2024
@ST-DDT ST-DDT added this to the v9.0 milestone May 18, 2024
@ST-DDT ST-DDT requested review from a team May 18, 2024 12:19
@ST-DDT ST-DDT self-assigned this May 18, 2024
@netlify
Copy link
Copy Markdown

netlify Bot commented May 18, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 425f1c7
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/664f69cdbae75f00089e2dbc
😎 Deploy Preview https://deploy-preview-2910.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.95%. Comparing base (b97984c) to head (425f1c7).

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2910   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files        2977     2977           
  Lines      215421   215421           
  Branches      948      951    +3     
=======================================
+ Hits       215327   215333    +6     
+ Misses         94       88    -6     
Files Coverage Δ
src/locales/base/system/directory_paths.ts 100.00% <ø> (ø)
src/locales/base/system/index.ts 100.00% <100.00%> (ø)
src/locales/base/system/mime_types.ts 100.00% <ø> (ø)
src/locales/en/science/chemical_element.ts 100.00% <ø> (ø)
src/locales/en/science/index.ts 100.00% <100.00%> (ø)
src/locales/eo/science/chemical_element.ts 100.00% <ø> (ø)
src/locales/eo/science/index.ts 100.00% <100.00%> (ø)
src/locales/nb_NO/science/chemical_element.ts 100.00% <ø> (ø)
src/locales/nb_NO/science/index.ts 100.00% <100.00%> (ø)
src/locales/pl/science/chemical_element.ts 100.00% <ø> (ø)
... and 5 more

... and 1 file with indirect coverage changes

Comment thread src/definitions/system.ts
Comment thread src/definitions/system.ts
@xDivisionByZerox
Copy link
Copy Markdown
Member

Why do we couple file anme and property key anyway? The propblem I have right now is that we have locale entries which might be lists of objects which intern have property keys as well. These are not considered in the PR. For example, look at en/airline/airline which is types as a list of Airline. It has the property iataCode, which is camelCase. So what I can do is faker.helpers.fake('airline.airline.iataCode') which contradictes the statement from the migration guide:

All our locale data now use the snake_case naming scheme.

@ST-DDT
Copy link
Copy Markdown
Member Author

ST-DDT commented May 18, 2024

Why do we couple file name and property key anyway?

Because the index files that merge them are generated.

We could transform the property keys in between, but it would be a massive breaking change. Sure we could do that, but we should talk about it first.

@ST-DDT
Copy link
Copy Markdown
Member Author

ST-DDT commented May 18, 2024

If you are just concerned about the wording, I can change it to refer to locale categories and entries specifically.

@xDivisionByZerox
Copy link
Copy Markdown
Member

If you are just concerned about the wording, I can change it to refer to locale categories and entries specifically.

Thats propably it for me. Alternativly we could actually make all properties snake_case. The definition properties which are not "correctly" cased right now (ignoring the ones from this PR) are:

  • airline.airline.iataCode
  • airline.airport.iataCode
  • internet.http_status_code.clientError
  • internet.http_status_code.serverError
  • science.chemical_element.atomicNumber

@ST-DDT ST-DDT requested review from a team and xDivisionByZerox May 20, 2024 20:00
@ST-DDT ST-DDT requested review from a team and matthewmayer May 23, 2024 16:11
@ST-DDT ST-DDT merged commit 558b959 into next May 25, 2024
@ST-DDT ST-DDT deleted the refactor/locale/snake-case branch May 25, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Cannot be merged when next version is not a major release c: locale Permutes locale definitions c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs p: 1-normal Nothing urgent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants