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: fixed inconsitency in @see usage in jsdoc #1224

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

hankucz
Copy link
Contributor

@hankucz hankucz commented Aug 3, 2022

No description provided.

@hankucz hankucz requested a review from a team August 3, 2022 19:04
@hankucz hankucz requested a review from a team as a code owner August 3, 2022 19:04
@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #1224 (7171c92) into main (5979f82) will increase coverage by 0.00%.
The diff coverage is 78.57%.

@@           Coverage Diff           @@
##             main    #1224   +/-   ##
=======================================
  Coverage   99.62%   99.62%           
=======================================
  Files        2155     2155           
  Lines      236900   236900           
  Branches      990      991    +1     
=======================================
+ Hits       236012   236015    +3     
+ Misses        867      864    -3     
  Partials       21       21           
Impacted Files Coverage Δ
src/definitions/finance.ts 0.00% <0.00%> (ø)
src/definitions/hacker.ts 0.00% <0.00%> (ø)
src/definitions/phone_number.ts 0.00% <0.00%> (ø)
src/modules/address/index.ts 99.81% <100.00%> (ø)
src/modules/company/index.ts 100.00% <100.00%> (ø)
src/modules/image/providers/unsplash.ts 95.32% <100.00%> (ø)
src/modules/phone/index.ts 100.00% <100.00%> (ø)
src/modules/internet/user-agent.ts 82.53% <0.00%> (+0.79%) ⬆️

@ejcheng ejcheng added the c: docs Improvements or additions to documentation label Aug 3, 2022
@ejcheng ejcheng added this to the v7 - Current Major milestone Aug 3, 2022
src/definitions/finance.ts Show resolved Hide resolved
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether all methods need the empty ().

@Shinigami92
Copy link
Member

Shinigami92 commented Aug 4, 2022

I'm not sure whether all methods need the empty ().

How did we do it in the rest of the codebase and if it's mixed please let us decide one, document that and use it consistently

Maybe your magic hands can even write a test for that?

@ST-DDT ST-DDT enabled auto-merge (squash) August 4, 2022 18:06
@ST-DDT
Copy link
Member

ST-DDT commented Aug 4, 2022

I will write a test after this PR is merged.

@ST-DDT ST-DDT merged commit 6321665 into faker-js:main Aug 4, 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
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants