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

refactor(locale): rename company affix files #2975

Merged
merged 5 commits into from
Jun 26, 2024
Merged

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Jun 23, 2024

Fixes #1574

This PR renames the company prefix and suffix files to category/legal_entity_type respectively.

@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 m: company Something is referring to the company module labels Jun 23, 2024
@ST-DDT ST-DDT added this to the v9.0 milestone Jun 23, 2024
@ST-DDT ST-DDT self-assigned this Jun 23, 2024
Copy link

netlify bot commented Jun 23, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 494f443
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/667bc5af4be3e600086c20a5
😎 Deploy Preview https://deploy-preview-2975.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.

Copy link

codecov bot commented Jun 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.96%. Comparing base (8134b33) to head (1ddc1e2).

Current head 1ddc1e2 differs from pull request most recent head 494f443

Please upload reports for the commit 494f443 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff            @@
##             next    #2975    +/-   ##
========================================
  Coverage   99.95%   99.96%            
========================================
  Files        2989     2984     -5     
  Lines      216081   216057    -24     
  Branches      596      949   +353     
========================================
- Hits       215980   215975     -5     
+ Misses        101       82    -19     
Files Coverage Δ
src/locales/af_ZA/company/index.ts 100.00% <100.00%> (ø)
src/locales/af_ZA/company/legal_entity_type.ts 100.00% <ø> (ø)
src/locales/az/company/index.ts 100.00% <100.00%> (ø)
src/locales/az/company/legal_entity_type.ts 100.00% <ø> (ø)
src/locales/az/company/name_pattern.ts 100.00% <100.00%> (ø)
src/locales/cs_CZ/company/index.ts 100.00% <100.00%> (ø)
src/locales/cs_CZ/company/legal_entity_type.ts 100.00% <ø> (ø)
src/locales/cs_CZ/company/name_pattern.ts 100.00% <100.00%> (ø)
src/locales/da/company/index.ts 100.00% <100.00%> (ø)
src/locales/da/company/legal_entity_type.ts 100.00% <ø> (ø)
... and 100 more

... and 2 files with indirect coverage changes

@ST-DDT
Copy link
Member Author

ST-DDT commented Jun 23, 2024

I was unable to identify which part is the category or legal_entity_type in lv and ru.
Maybe they have the legal_entity_type sometimes as prefix and sometimes as suffix?

@serembon @salixzs: Are you able to clarify this?

I contacted you as creators of these commits:

@ST-DDT ST-DDT requested review from a team June 23, 2024 21:34
@ST-DDT ST-DDT marked this pull request as ready for review June 23, 2024 21:35
@serembon
Copy link
Contributor

@ST-DDT hello! prefix can only be legal_entity_type

@ST-DDT
Copy link
Member Author

ST-DDT commented Jun 23, 2024

@ST-DDT hello! prefix can only be legal_entity_type

@serembon Thanks for your fast response. What would you call the suffix in that case?

@serembon
Copy link
Contributor

serembon commented Jun 23, 2024

This is part of the company name. It is necessary to save this name_patterns for generating a name from parts

{{company.suffix}}{{company.suffix}}'
{{company.suffix}}{{company.suffix}}{{company.suffix}}

If you use only one part, the company name will not be similar to the real one.

@ST-DDT
Copy link
Member Author

ST-DDT commented Jun 23, 2024

So the suffix is actually the suffix in this case?

@serembon
Copy link
Contributor

So the suffix is actually the suffix in this case?

Right

@ST-DDT ST-DDT enabled auto-merge (squash) June 26, 2024 07:39
@ST-DDT ST-DDT merged commit 49d7119 into next Jun 26, 2024
21 checks passed
@ST-DDT ST-DDT deleted the refactor/company/affix branch June 26, 2024 07:42
@xDivisionByZerox
Copy link
Member

I just noticed that we missed the update of CompanyDefinition in this PR.
@ST-DDT Was this intended, since "suffix" was never part of it in the first place?

@ST-DDT
Copy link
Member Author

ST-DDT commented Jun 27, 2024

@ST-DDT Was this intended, since "suffix" was never part of it in the first place?

It was never part of it in the first place (It is only usd by fake patterns).

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 m: company Something is referring to the company module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename company.suffix definition to company.legal_entity_type
5 participants