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

docs: add hint that the generated data might be real ones #959

Merged
merged 4 commits into from
Jun 18, 2022

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented May 15, 2022

We should warn our users that the generated data may be working ones and not the well known 555 dummy ones.

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent labels May 15, 2022
@ST-DDT ST-DDT requested review from a team May 15, 2022 12:47
@ST-DDT ST-DDT self-assigned this May 15, 2022
@codecov
Copy link

codecov bot commented May 15, 2022

Codecov Report

Merging #959 (0b7b106) into main (045b8d6) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #959   +/-   ##
=======================================
  Coverage   99.64%   99.65%           
=======================================
  Files        2146     2146           
  Lines      230380   230380           
  Branches      977      981    +4     
=======================================
+ Hits       229563   229585   +22     
+ Misses        796      774   -22     
  Partials       21       21           
Impacted Files Coverage Δ
src/modules/internet/user-agent.ts 92.75% <0.00%> (+6.37%) ⬆️

@Shinigami92
Copy link
Member

Random thought: why not for:

  • email
  • iban
  • vrm
  • vim
  • bitcoin address
  • ipv4/6
  • ...

I'm not sure if this hint is helpful 🤔

@ST-DDT
Copy link
Member Author

ST-DDT commented May 15, 2022

We might want to add a general hint at some point (or maybe actually generate fake numbers).

But I created this one to address an issue raised in Discord:
https://discord.com/channels/929487054990110771/929544565348777984/974285076064002148

@Shinigami92
Copy link
Member

We might want to add a general hint at some point (or maybe actually generate fake numbers).

But I created this one to address an issue raised in Discord: discord.com/channels/929487054990110771/929544565348777984/974285076064002148

IMO we should just add the hint now globally. For me it doesn't make really sense to add it here specifically just because one person asked in discord if values are real or not.
Some day just the next person would come up and ask for another function and so on 🤷
I also would say it is not in scope for Faker at all to generate or prevent real data. Just data that "looks" real and if they are "accidentally" real, so they are.

@Shinigami92 Shinigami92 added the s: needs decision Needs team/maintainer decision label May 18, 2022
prisis
prisis previously approved these changes May 23, 2022
@pkuczynski
Copy link
Member

I agree with @Shinigami92 that it should be done as a general hint and not for a specific function. The whole concept of faker is to generate data that looks real, so as he pointed out, they might be actually real!

@ST-DDT
Copy link
Member Author

ST-DDT commented May 24, 2022

Yeah, I will think of a way to put it somewhere else.

@ST-DDT ST-DDT force-pushed the docs/phone/phoneNumber-real-number-warning branch from defba77 to 0773611 Compare June 17, 2022 07:54
@ST-DDT ST-DDT requested a review from a team June 17, 2022 07:54
@ST-DDT ST-DDT changed the title docs: add hint that phone number might return a working one docs: add hint that the generated data might be valid ones Jun 17, 2022
@ST-DDT ST-DDT changed the title docs: add hint that the generated data might be valid ones docs: add hint that the generated data might be real ones Jun 17, 2022
README.md Outdated Show resolved Hide resolved
@ST-DDT ST-DDT requested a review from a team June 18, 2022 15:17
@Shinigami92 Shinigami92 enabled auto-merge (squash) June 18, 2022 15:24
@Shinigami92 Shinigami92 merged commit ab2ed23 into main Jun 18, 2022
@Shinigami92 Shinigami92 deleted the docs/phone/phoneNumber-real-number-warning branch June 18, 2022 15:30
Minozzzi pushed a commit to Minozzzi/faker that referenced this pull request Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants