Skip to content
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

feat: add base locale #1748

Merged
merged 15 commits into from
Mar 29, 2023
Merged

feat: add base locale #1748

merged 15 commits into from
Mar 29, 2023

Conversation

Shinigami92
Copy link
Member

closes #1441

this adds a global locale that can later be used as the fallback for everything
it contains just generic data not related to a locale specifically, like e.g. css_space which are the same in every locale in the world

@Shinigami92 Shinigami92 added the c: locale Permutes locale definitions label Jan 17, 2023
@Shinigami92 Shinigami92 self-assigned this Jan 17, 2023
@Shinigami92 Shinigami92 linked an issue Jan 17, 2023 that may be closed by this pull request
@github-actions
Copy link
Contributor

Uncommitted changes were detected after runnning generate:* commands.
Please run pnpm run generate:locales, pnpm run generate:api-docs, and pnpm run test -u to generate/update the related files, and commit them.

@Shinigami92
Copy link
Member Author

As we have currently not the possibility to declare multiple stacked localeFallback, this PR need to be done in conjunction with

there we can finally use e.g. locale: [en_IN, en, global],

Copy link
Member Author

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is (IMO) currently blocked by #1735, but @ST-DDT please let me know if this is going in the right direction as you expected

src/definitions/color.ts Outdated Show resolved Hide resolved
src/locale/global.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/definitions/color.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT added s: on hold Blocked by something or frozen to avoid conflicts p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs labels Jan 17, 2023
@Shinigami92
Copy link
Member Author

Shinigami92 commented Jan 17, 2023

Okay will revert the color, and potentially convert the color related things to enums in another PR
Like SexType.

But I can proceed with stuff like emoji and other already locales that are in en right now (?)

@Shinigami92
Copy link
Member Author

TBD

Maybe move to global?

  • finance->currency
  • science->chemicalElement
  • science->unit

Maybe move to another locale?

  • location->state(_abbr) -> en US

Maybe purge?

  • business->credit_card_types
  • internet->avatar_uri

ST-DDT
ST-DDT previously approved these changes Jan 27, 2023
@Shinigami92
Copy link
Member Author

@ST-DDT thx for you approval, but before #1735 is not merged, we cannot proceed with this PR as well

@Shinigami92 Shinigami92 force-pushed the 1441-add-global-locale branch from b0cccb2 to ef5c56c Compare January 29, 2023 21:44
@ST-DDT ST-DDT added needs rebase There is a merge conflict s: on hold Blocked by something or frozen to avoid conflicts and removed s: on hold Blocked by something or frozen to avoid conflicts labels Mar 7, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Mar 7, 2023

#1735 is merged, so this is no longer blocked.

Most/All(?) of the merge conflicts can be solved by running pnpm run preflight.

@ST-DDT
Copy link
Member

ST-DDT commented Mar 7, 2023

TODO:

  • Add global to the generated fallback locales in the Faker constructor args and adjust the generate:locales script to handle the global locale properly.

src/locale/global.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 force-pushed the 1441-add-global-locale branch from 7bb31d8 to 07a2054 Compare March 7, 2023 15:07
@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #1748 (9177b99) into next (de47aaa) will decrease coverage by 0.01%.
The diff coverage is 95.35%.

❗ Current head 9177b99 differs from pull request most recent head 1eba502. Consider uploading reports for the commit 1eba502 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #1748      +/-   ##
==========================================
- Coverage   99.63%   99.62%   -0.01%     
==========================================
  Files        2450     2458       +8     
  Lines      239748   239905     +157     
  Branches     1276     1276              
==========================================
+ Hits       238864   239004     +140     
- Misses        862      878      +16     
- Partials       22       23       +1     
Impacted Files Coverage Δ
src/locale/global.ts 0.00% <0.00%> (ø)
src/locales/base/color/space.ts 100.00% <ø> (ø)
src/locales/base/database/collation.ts 100.00% <ø> (ø)
src/locales/base/database/engine.ts 100.00% <ø> (ø)
src/locales/base/database/type.ts 100.00% <ø> (ø)
src/locales/base/hacker/abbreviation.ts 100.00% <ø> (ø)
src/locales/base/internet/emoji.ts 100.00% <ø> (ø)
src/locales/base/internet/http_status_code.ts 100.00% <ø> (ø)
src/locales/base/location/country_code.ts 100.00% <ø> (ø)
src/locales/base/location/time_zone.ts 100.00% <ø> (ø)
... and 78 more

... and 1 file with indirect coverage changes

@ST-DDT
Copy link
Member

ST-DDT commented Mar 23, 2023

Poll Decision

  • base has won

@matthewmayer
Copy link
Contributor

Rename the PR title also?

image

@Shinigami92 Shinigami92 changed the title feat: add global locale feat: add base locale Mar 23, 2023
ST-DDT
ST-DDT previously approved these changes Mar 23, 2023
@ST-DDT ST-DDT requested a review from a team March 23, 2023 19:00
docs/guide/localization.md Outdated Show resolved Hide resolved
Co-authored-by: Matt Mayer <152770+matthewmayer@users.noreply.github.com>
@ST-DDT ST-DDT requested review from matthewmayer and a team March 28, 2023 09:50
@ST-DDT ST-DDT requested a review from a team March 28, 2023 11:14
@Shinigami92 Shinigami92 enabled auto-merge (squash) March 29, 2023 08:21
@Shinigami92 Shinigami92 merged commit f890d62 into next Mar 29, 2023
@Shinigami92 Shinigami92 deleted the 1441-add-global-locale branch March 29, 2023 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add global locale
3 participants