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

chore: remove helpers.userCard() from README.md #1013

Merged
merged 5 commits into from
May 28, 2022

Conversation

andrecastelo
Copy link
Contributor

Related to #528 and #543. Since the helpers.*Card methods were deprecated we need to remove the example from README.md.

@andrecastelo andrecastelo requested a review from a team May 27, 2022 14:46
@andrecastelo andrecastelo requested a review from a team as a code owner May 27, 2022 14:46
@Shinigami92
Copy link
Member

I also had this in mind, but we may want to replace it with e.g. the arrayElement function, so at least there is one example.

Alternatively we could remove the whole section and just refer to the docs 🤔

@Shinigami92 Shinigami92 added the c: docs Improvements or additions to documentation label May 27, 2022
@Shinigami92 Shinigami92 added this to the v7 - Current Major milestone May 27, 2022
@andrecastelo
Copy link
Contributor Author

I also had this in mind, but we may want to replace it with e.g. the arrayElement function, so at least there is one example.

Alternatively we could remove the whole section and just refer to the docs 🤔

True, true. IMO it is better if we leave the example there - it helps with discoverability, and then the user can look into the docs for more helper methods. Also the docs link is not very prominent (I think would be nice if the link was moved to the header, next to the stackblitz link).

@import-brain import-brain added the p: 1-normal Nothing urgent label May 27, 2022
@codecov
Copy link

codecov bot commented May 27, 2022

Codecov Report

Merging #1013 (422cac2) into main (d463ff8) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1013      +/-   ##
==========================================
- Coverage   99.67%   99.67%   -0.01%     
==========================================
  Files        2115     2115              
  Lines      227025   227025              
  Branches      983      982       -1     
==========================================
- Hits       226280   226277       -3     
- Misses        725      728       +3     
  Partials       20       20              
Impacted Files Coverage Δ
src/modules/finance/index.ts 99.31% <0.00%> (-0.69%) ⬇️

@andrecastelo
Copy link
Contributor Author

Added faker.helpers.arrayElement() example, and added a link to the Documentation at the top of README.md

import-brain
import-brain previously approved these changes May 28, 2022
@Shinigami92 Shinigami92 enabled auto-merge (squash) May 28, 2022 12:28
@Shinigami92 Shinigami92 merged commit ee65c37 into faker-js:main May 28, 2022
Minozzzi pushed a commit to Minozzzi/faker that referenced this pull request Jul 19, 2022
@xDivisionByZerox xDivisionByZerox added the m: helpers Something is referring to the helpers module label Jul 28, 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 m: helpers Something is referring to the helpers module p: 1-normal Nothing urgent
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants