feat: new method faker.word.any

[pkuczynski]

Fixes #276

Fixes #339

[codecov[bot]]

Codecov Report

Merging #714 (de9f2d6) into next (666ff02) will decrease coverage by 0.00%. The diff coverage is 92.40%.

Additional details and impacted files

@@            Coverage Diff             @@
##             next     #714      +/-   ##
==========================================
- Coverage   99.64%   99.63%   -0.01%     
==========================================
  Files        2214     2214              
  Lines      238741   238813      +72     
  Branches     1023     1029       +6     
==========================================
+ Hits       237885   237951      +66     
- Misses        835      841       +6     
  Partials       21       21              

Impacted Files	Coverage Δ
src/modules/word/index.ts	98.47% <92.00%> (-1.53%)	:arrow_down:
src/modules/helpers/index.ts	98.51% <100.00%> (ø)
src/modules/system/index.ts	100.00% <100.00%> (ø)

[pkuczynski]

@Shinigami92 anything else missing to get this merged?

[Shinigami92]

Yes, all the v6.1 targeted things to be merged I assume 🤔

[pkuczynski]

Yes, all the v6.1 targeted things to be merged I assume 🤔

Not sure if we should be so stick to the 6.1 is only bug fixes as keeping PRs open for longer time means potential conflicts and maintenance, so I would be in favor of merging things as soon as they ready...

[ST-DDT]

Yes, all the v6.1 targeted things to be merged I assume 🤔

Not sure if we should be so stick to the 6.1 is only bug fixes as keeping PRs open for longer time means potential conflicts and maintenance

That is true. Is is a small feature, requiring low effort for maintaining the PR from you.

so I would be in favor of merging things as soon as they ready...

If we merge this one, then we have to explain why we didn't merge other features. Sure we could start merging them all, but then everyone would start creating new PRs with new super fancy features and we have to somehow interweave the required bug fix merges with the nice to have feature requests. Also people tend to prefer adding features over fixing bugs.

We use the milestones as a means to communicate the order we discussed on when to tackle things/fix/add... This feature being in v6.2 has been discussed/communicated a few months ago, we don't want to discourage anyone in contributing to the project, so we don't ban PRs for future milestones, but I hope you can understand that this may cause you additional work to maintain them for the time being.

We hope to finish all fixes/PRs for 6.1 by this Monday evening, so you don't have to maintain it forever either.

[pkuczynski]

Fair enough :)

[ST-DDT]

To consider: Handle "empty" locale This method should at least not behave worse than the other word methods. Ref: https://github.com/faker-js/faker/issues/770#issuecomment-1087249521

[xDivisionByZerox]

This PR is really outdated. I guess its easier to close this and then recreate it on the current main branch. @pkuczynski Are you ok with this? Do you want to do that? Otherwise, I can offer you help from my side.

[ST-DDT]

I updated this branch to retain the original contribution references to @pkuczynski .

[ST-DDT]

Test failures -> https://github.com/faker-js/faker/issues/1317 + https://github.com/faker-js/faker/issues/1318

[Shinigami92]

To be fair we would need to split this PR in two parts if @xDivisionByZerox's recommendation to have simple responsibilities by a PR counts for for this PR as well

So one PR introducing the new method sample And one for deprecating the old method(s)

@xDivisionByZerox's point was also about that the commit history is more clean and if e.g. something need to be reverted, the commit in the default branch history can be better linked

[ST-DDT]

@Shinigami92 I think you are mistaken here. If we replace one thing with another, then we do it in one step (and always have been). (Same as we did with renaming the name->person module, address->location, fake->helpers, phoneNumber->number, ...)

[Shinigami92]

@Shinigami92 I think you are mistaken here. If we replace one thing with another, then we do it in one step (and always have been). (Same as we did with renaming the name->person module, address->location, fake->helpers, phoneNumber->number, ...)

But is this really a one to one replacement? I thought the code differ much from what the original code did. The wordMethods from the old method had MUCH more possible values. You can also see that the random.word snapshot results did change drastically (not only one offset but totally different results)

[ST-DDT]

So you are referring to not replacing the old implementation itself?

[Shinigami92]

referring

yes

like @xDivisionByZerox would say: this PR is doing two things

1. moving the old method to a new home
2. changing the implementation of that new method

[ST-DDT]

I strongly disagree with moving the method implementation. 🙅 The old implementation is a list of random methods that have very little in common with "words". The word module should not gain dependencies on any external module (expect for the current helpers one).

AFAICT we have two choices here:

• Deprecate the old method and replace its implementation with a delegate to the new words.sample method.
• Deprecate the old method and retain it as it is till removal.

(Potentially in a separate PR from the one introducing the word.sample method).

[ST-DDT]

Team decision

• This PR will only add the sample method.
• The random.word will be deprecated in a separate PR but will retain the original implementation

[ST-DDT]

Done. Ready for Review.