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.

Sign up for GitHub

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

Jump to bottom

Use singular locale definition keys #2911

Closed
ST-DDT opened this issue May 18, 2024 · 4 comments · Fixed by #2932
Closed

Use singular locale definition keys #2911

ST-DDT opened this issue May 18, 2024 · 4 comments · Fixed by #2932
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 s: accepted Accepted feature / Confirmed bug
Milestone
v9.0

Comments

@ST-DDT
Copy link
Member

ST-DDT commented May 18, 2024

Followup for #2910

Currently we have some locale files that use plural naming, and some that use singular naming.

singular plural
faker.definitions.science.chemical_element faker.definitions.system.directory_paths

We should decide on one naming scheme and stick to that.

Currently, most definitions are singular.
@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 self-assigned this May 18, 2024
@ST-DDT
Copy link
Member Author

ST-DDT commented May 18, 2024

Blocked by #2910

@ST-DDT ST-DDT added s: on hold Blocked by something or frozen to avoid conflicts s: needs decision Needs team/maintainer decision labels May 18, 2024
@ST-DDT ST-DDT mentioned this issue May 18, 2024
Merged
@matthewmayer
Copy link
Contributor

I don't think it's that important to rename them all, but if we are renaming from camel case to snake case may as well change at the same time.

@ST-DDT ST-DDT removed the s: on hold Blocked by something or frozen to avoid conflicts label May 30, 2024
@ST-DDT
Copy link
Member Author

ST-DDT commented May 30, 2024

Currently the only plural names are the following:

  • lorem.words
  • system.directory_paths
  • system.mime_types

The rest is singular.

@ST-DDT
Copy link
Member Author

ST-DDT commented May 30, 2024

Team Decision

  • We will name the locale data to singular.
  • Because if they are used in fake they refer to only one element of that list
  • Because of historic design decisions (pre v6)/backwards compatibility.

@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision labels May 30, 2024
SaurabhXRT added a commit to SaurabhXRT/faker-xrt that referenced this issue Jun 1, 2024
@SaurabhXRT
used singular local keys for issue faker-js#2911
9e06916
@SaurabhXRT SaurabhXRT mentioned this issue Jun 1, 2024
Merged
@ST-DDT ST-DDT linked a pull request Jun 1, 2024 that will close this issue
refactor(locale)!: use singular locale definition keys #2932
Merged
@ST-DDT ST-DDT removed their assignment Jun 2, 2024
matthewmayer added a commit to SaurabhXRT/faker-xrt that referenced this issue Jun 5, 2024
@matthewmayer
Merge branch 'next' into Use-singular-locale-definition-keys-faker-js…
69c0f25 
…#2911
SaurabhXRT added a commit to SaurabhXRT/faker-xrt that referenced this issue Jun 6, 2024
@SaurabhXRT
Merge branch 'Use-singular-locale-definition-keys-faker-js#2911' of h…
41eb5bd 
…ttps://github.com/SaurabhXRT/faker-xrt into Use-singular-locale-definition-keys-faker-js#2911
ST-DDT added a commit to SaurabhXRT/faker-xrt that referenced this issue Jun 10, 2024
@ST-DDT
Merge branch 'next' into Use-singular-locale-definition-keys-faker-js…
a64a3aa 
…#2911
ST-DDT added a commit to SaurabhXRT/faker-xrt that referenced this issue Jun 10, 2024
@ST-DDT
Merge branch 'next' into Use-singular-locale-definition-keys-faker-js…
cb24833 
…#2911
ST-DDT added a commit to SaurabhXRT/faker-xrt that referenced this issue Jun 12, 2024
@ST-DDT
Merge branch 'next' into Use-singular-locale-definition-keys-faker-js…
854f57c 
…#2911
xDivisionByZerox added a commit to SaurabhXRT/faker-xrt that referenced this issue Jun 13, 2024
@xDivisionByZerox
Merge branch 'next' into Use-singular-locale-definition-keys-faker-js…
ca753dc 
…#2911
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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 s: accepted Accepted feature / Confirmed bug
Projects
None yet
Milestone
v9.0
Development

Successfully merging a pull request may close this issue.

refactor(locale)!: use singular locale definition keys SaurabhXRT/faker-xrt
2 participants
@matthewmayer @ST-DDT