refactor(person)!: rename name module

[Shinigami92]

closes #1343

[Shinigami92]

Blocked by:

• refactor!: cleanup deprecations #1440

[Shinigami92]

@ST-DDT I'm not sure if we want to support faker.helpers.fake('{{name.firstName}} or {{name.first_name}}')
It might be really hard for us to maintain that in the long run
We also would need to write extra code to support that but in a way so we don't duplicate the name definitions
So you have any ideas? Or should we just make a hard breaking change?

[Shinigami92]

Okay, this is now in a done / ready for review state, but I want to first merge the cleanup deprecations as of that will reduce some changes.
So until then, this PR will stay in draft.

[codecov]

Codecov Report

Merging #1445 (086c23c) into next (90b9c5c) will increase coverage by 0.01%.
The diff coverage is 98.64%.

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #1445      +/-   ##
==========================================
+ Coverage   99.62%   99.63%   +0.01%     
==========================================
  Files        2164     2164              
  Lines      236815   236854      +39     
  Branches     1003     1003              
==========================================
+ Hits       235916   235984      +68     
+ Misses        877      849      -28     
+ Partials       22       21       -1     

Impacted Files	Coverage Δ
src/definitions/definitions.ts	0.00% <0.00%> (ø)
src/definitions/index.ts	0.00% <0.00%> (ø)
src/definitions/person.ts	0.00% <0.00%> (ø)
src/locales/af_ZA/person/female_first_name.ts	100.00% <ø> (ø)
src/locales/af_ZA/person/first_name.ts	100.00% <ø> (ø)
src/locales/af_ZA/person/last_name.ts	100.00% <ø> (ø)
src/locales/af_ZA/person/male_first_name.ts	100.00% <ø> (ø)
src/locales/ar/person/female_first_name.ts	100.00% <ø> (ø)
src/locales/ar/person/first_name.ts	100.00% <ø> (ø)
src/locales/ar/person/last_name.ts	100.00% <ø> (ø)
... and 649 more

[ST-DDT]

I don't know why this works, but it works.

[matthewmayer]

in addition to the new faker.person tests, why not keep the existing tests for the faker.name module ?

test/__snapshots__/name.spec.ts.snap
test/name.spec.ts

That will help ensure no unexpected regressions/breaking changes.

[Shinigami92]

in addition to the new faker.person tests, why not keep the existing tests for the faker.name module ?

test/__snapshots__/name.spec.ts.snap
test/name.spec.ts

That will help ensure no unexpected regressions/breaking changes.

Because it is just an alias, behind it there is exactly the same code, so no need to test it twice.

[xDivisionByZerox]

I'm a bit lost, shouldn't this follow our normal deprecation workflow? As now a user has no time-window to migrate, but have to rework their codebase. Is this desired even if this will be in a breaking change?

[xDivisionByZerox]

I'm a bit lost, shouldn't this follow our normal deprecation workflow? As now a user has no time-window to migrate, but have to rework their codebase. Is this desired even if this will be in a breaking change?

Sorry, too many files. Didn't saw the getter in the Faker class. So if this follows the typical deprecation workflow, we need to keep the tests as @matthewmayer suggested. Even is if is just an alias, how do you confirm that it is that way? The test would also catch breaking changes in the NameModule which were designed to only affect the PersonModule.

[ST-DDT]

So if this follows the typical deprecation workflow, we need to keep the tests as @matthewmayer suggested. Even is if is just an alias, how do you confirm that it is that way? The test would also catch breaking changes in the NameModule which were designed to only affect the PersonModule.

I'm not sure whether I get you right.
If we duplicate the tests for faker.name, then this would only prevent breaking changes if we freeze the name tests until after v8.0.0.
However, the same applies to the PersonModule.
If we treat it like an existing module and only change things after a deprecation phase, then there is no need for the extra tests.

[ST-DDT]

TODO: Create an issue to remove the special handling in v10.0 after this is merged (to get the final line numbers and commit references).

[import-brain]

LGTM :)