Skip to content

chore: rename phone_number to phone #340

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

Merged
merged 1 commit into from
Jan 29, 2022

Conversation

pkuczynski
Copy link
Member

Every other file is named like the class, except this one:

class: Phone
filename: phone_number

I aligned this with other files and simply renamed this one to match the class name.

@pkuczynski pkuczynski requested a review from a team as a code owner January 28, 2022 20:21
@pkuczynski pkuczynski added the c: chore PR that doesn't affect the runtime behavior label Jan 28, 2022
@pkuczynski pkuczynski added this to the v6.0 - Project stability milestone Jan 28, 2022
@pkuczynski
Copy link
Member Author

The same thing should possibly happen to locales. Let me know if I should do it (in this or separate PR)

@Shinigami92
Copy link
Member

Where were not sure right now if the fake function somehow dynamically requests data with a pattern that uses phone_number
I would like to first update the tests for that.
Also I'm not sure if this counts to the project stability 🤔
I will move it to the parallel milestone

@pkuczynski
Copy link
Member Author

fake is referring to classes, not file names:

https://github.com/faker-js/faker/blob/main/test/fake.spec.ts#L7

and the class itself is using definitions in this way:

https://github.com/faker-js/faker/blob/main/src/phone_number.ts#L36

So I think its pretty safe change and its always good to verify via tests...

@ST-DDT ST-DDT requested a review from a team January 28, 2022 23:13
@Shinigami92 Shinigami92 merged commit e453c93 into faker-js:main Jan 29, 2022
@pkuczynski pkuczynski deleted the phone-rename-file branch January 29, 2022 23:15
bmenant pushed a commit to bmenant/faker that referenced this pull request Mar 11, 2022
demipel8 pushed a commit to demipel8/faker that referenced this pull request Mar 11, 2022
demipel8 pushed a commit to demipel8/faker that referenced this pull request Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: chore PR that doesn't affect the runtime behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants