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

fix(generate:locale): make the definition types extendible #915

Merged
merged 4 commits into from May 4, 2022

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented May 3, 2022

I messed up and removed code (#910), that was used by our scripts, so now I have to adjust the scripts to work without them.

We could revert that commit, but we want to get ride of it in the long run anyway.

I based this PR on #728 (#729) and adjusted the locale definitions to be Partial by default.
All usages were already partial and optional anyway:

export type LocaleDefinition = {
/**
* The name of the language.
*/
title: string;
separator?: string;
} & {
// Known modules
[module in keyof Definitions]?: Partial<Definitions[module]>;
} & {

I recommend, to review only the first commit/changes to the script and definitions as the second commit is only the execution of the script.

I will try to create a separate PR, that will check the scripts at least type wise (Issue #917).

ST-DDT and others added 2 commits May 3, 2022 22:18
@ST-DDT ST-DDT added c: bug Something isn't working p: 2-high Fix main branch labels May 3, 2022
@ST-DDT ST-DDT requested review from a team May 3, 2022 20:32
@ST-DDT ST-DDT self-assigned this May 3, 2022
@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #915 (f2bd56f) into main (f1dba1b) will decrease coverage by 0.01%.
The diff coverage is 81.34%.

@@            Coverage Diff             @@
##             main     #915      +/-   ##
==========================================
- Coverage   99.67%   99.66%   -0.02%     
==========================================
  Files        1957     1957              
  Lines      209768   209797      +29     
  Branches      885      885              
==========================================
  Hits       209084   209084              
- Misses        664      693      +29     
  Partials       20       20              
Impacted Files Coverage Δ
src/definitions/address.ts 0.00% <0.00%> (ø)
src/definitions/animal.ts 0.00% <0.00%> (ø)
src/definitions/commerce.ts 0.00% <0.00%> (ø)
src/definitions/company.ts 0.00% <0.00%> (ø)
src/definitions/database.ts 0.00% <0.00%> (ø)
src/definitions/date.ts 0.00% <0.00%> (ø)
src/definitions/definitions.ts 0.00% <0.00%> (ø)
src/definitions/finance.ts 0.00% <0.00%> (ø)
src/definitions/hacker.ts 0.00% <0.00%> (ø)
src/definitions/index.ts 0.00% <0.00%> (ø)
... and 243 more

@Shinigami92 Shinigami92 requested a review from a team May 3, 2022 20:48
@Shinigami92 Shinigami92 mentioned this pull request May 4, 2022
@ST-DDT ST-DDT merged commit 984fbb4 into main May 4, 2022
@ST-DDT ST-DDT deleted the fix/locale-definitions branch May 4, 2022 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working p: 2-high Fix main branch
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants