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

Check modules and methods regarding their name and location #1346

Closed
ST-DDT opened this issue Sep 6, 2022 · 14 comments
Closed

Check modules and methods regarding their name and location #1346

ST-DDT opened this issue Sep 6, 2022 · 14 comments
Labels
breaking change Cannot be merged when next version is not a major release c: feature Request for new feature c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs p: 1-normal Nothing urgent

Comments

@ST-DDT
Copy link
Member

ST-DDT commented Sep 6, 2022

Clear and concise description of the problem

Some methods are misplaced or misnamed. The same is true for some module.

E.g. the string and number methods, or the address and name modules

Suggested solution

Check every module and method regarding their current name/location.

Alternative

No response

Additional context

No response

@ST-DDT ST-DDT added c: feature Request for new feature s: on hold Blocked by something or frozen to avoid conflicts p: 1-normal Nothing urgent c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs breaking change Cannot be merged when next version is not a major release labels Sep 6, 2022
@ST-DDT ST-DDT added this to the v8.0 - Module Re-Shuffling milestone Sep 6, 2022
@ST-DDT
Copy link
Member Author

ST-DDT commented Sep 6, 2022

@xDivisionByZerox
Copy link
Member

This should no longer be blocked. Correct, @ST-DDT?

@ST-DDT
Copy link
Member Author

ST-DDT commented Dec 22, 2022

@xDivisionByZerox Yeah, you are right. Do you wish to tackle this?

@ST-DDT ST-DDT removed the s: on hold Blocked by something or frozen to avoid conflicts label Dec 22, 2022
@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jan 29, 2023

Checklist from my side:

Module State
Animal
Color
Commerce
Company
Database
Datatype #1590
Date
  • I don't really know why we have between AND betweens
  • potentially month should be called monthName
Finance
Git
  • shortSha should probably be merged with commitSha and configurable via an argument
Hacker
  • ingverb should probably be called ingVerb (or generally a better name)
Helpers
Image
Internet
  • There is email AND exampleEmail (what is an example email?)
Location
Lorem
  • slug seems out of place for me here
  • There is paragraphs and text - aint that the same thing?
Music
Number Maybe a bit weird that int and hex use abbreviation while binary and octal don't, but ✅
Person
  • job* could fit in the CompanyModule as well
Phone
Random
  • Having a word function while we have an entire WordModule seems off (same for words)
  • locale should probably be located in the LocationModule
  • The RandomModule could be dropped entirely
Science
String
System
Vehicle
  • vin should probably be called identificationNumber for readability purposes
  • vrm should probably be called registrationMark for readability purposes
Word

@import-brain
Copy link
Member

Checklist from my side:

Module State
Animal ✅
Color ✅
Commerce ✅
Company

  • bs should probably be deprecated
  • bsAdjective should probably be merged into catchPhraseAdjective and deprecate
  • bsBuzz should probably be called buzzWord (is that correctly written?)
  • bsNoun should probably be merged into catchPhraseNoun and deprecated

Database ✅
Datatype #1590
Date

  • I don't really know why we have between AND betweens
  • potentially month should be called monthName

Finance

  • account should probably be called accountNumber
  • iban should perhaps be called IBAN
  • mask should be renamed to maskedNumber or be removed

Git

  • shortSha should probably be merged with commitSha and configurable via an argument

Hacker

  • ingverb should probably be called ingVerb (or generally a better name)

Helpers ✅
Image ✅
Internet

  • There is email AND exampleEmail (what is an example email?)

Location

  • city and cityName should probably be merged
  • nearbyGPSCoordinate perhaps could be redefined to coordinates
  • state and stateAbbr should probably be merged
  • streetAddress
  • Having street, streetAddress, streetName AND secondaryAddress is very confusing to me
  • street and streetName should probably be merged

Lorem

  • slug seems out of place for me here
  • There is paragraphs and text - aint that the same thing?

Music ✅
Number Maybe a bit weird that int and hex use abbreviation while binary and octal don't, but ✅
Person

  • job* could fit in the CompanyModule as well

Phone ✅
Random

  • Having a word function while we have an entire WordModule seems off (same for words)
  • locale should probably be located in the LocationModule
  • The RandomModule could be dropped entirely

Science ✅
String

System ✅
Vehicle

  • vin should probably be called identificationNumber for readability purposes
  • vrm should probably be called registrationMark for readability purposes

Word ✅

About lorem.slug, perhaps we could move it to a new method internet.slug? I feel like it would make more sense in internet. I assume we would have to deprecate lorem.slug first though

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jan 29, 2023

Team Decision: Company

bs should probably be deprecated

Yes!

bsAdjective should probably be merged into catchPhraseAdjective and deprecate
bsBuzz should probably be called buzzWord (is that correctly written?)
bsNoun should probably be merged into catchPhraseNoun and deprecated

Replace bs* with buzz* and improve JSDocs (buzz = manager language, catchPhrase = user language).

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jan 29, 2023

Team Decision: Date

I don't really know why we have between AND betweens

To be decided later.

potentially month should be called monthName

Sounds good. More information will be discussed in an individual issue.

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jan 29, 2023

Team Decision: Finance

account should probably be called accountNumber

Sounds good.

mask should be renamed to maskedNumber or be removed

Renaming sounds good.

iban should perhaps be called IBAN

Nope, this is naming convention. Will not be changed.

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jan 29, 2023

Team Decision Git

shortSha should probably be merged with commitSha and configurable via an argument

Sound good. Can be a length param.

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jan 29, 2023

Team Decision: Hacker

ingverb should probably be called ingVerb (or generally a better name)

We want to give this a more descriptive name but will discuss the details later. We can also consider removing but require more information for other languages.

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jan 29, 2023

Team Decision: Internet

There is email AND exampleEmail (what is an example email?)

rfc2606 - .example domain
We can merge both via an option flag in email and url.

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jan 29, 2023

Team Decision: Location

state and stateAbbr should probably be merged

Sounds good.

city and cityName should probably be merged
street and streetName should probably be merged
nearbyGPSCoordinate perhaps could be redefined to coordinates

Discuss all naming later.

Having street, streetAddress, streetName AND secondaryAddress is very confusing to me

Discuss later.


✅ We also want to merge zipCode and zipCodeByState. zipCode should then throw an error if the provided state argument is unknown.

@Shinigami92
Copy link
Member

It seems we have tackled all needed changes. If we want to change something additionally, we should open issues/PRs specifically.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Faker Roadmap May 6, 2023
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: feature Request for new feature c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs p: 1-normal Nothing urgent
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants