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: jsdoc for options #1644

Merged
merged 3 commits into from
Jan 18, 2023
Merged

docs: jsdoc for options #1644

merged 3 commits into from
Jan 18, 2023

Conversation

Shinigami92
Copy link
Member

ref #1586

@Shinigami92 Shinigami92 added the c: docs Improvements or additions to documentation label Dec 9, 2022
@Shinigami92 Shinigami92 added this to the v8.0 - Module Re-Shuffling milestone Dec 9, 2022
@Shinigami92 Shinigami92 self-assigned this Dec 9, 2022
@Shinigami92
Copy link
Member Author

@ST-DDT @xDivisionByZerox Before I move on copying over all the stuff from one place to another: is this going in the right direction, or do we need to re-phrase something?
Also did I missed something?
This is for now an example for the color module (the animal module didn't had options)

@codecov
Copy link

codecov bot commented Dec 9, 2022

Codecov Report

Merging #1644 (3282ce5) into next (0ac9989) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##             next    #1644     +/-   ##
=========================================
  Coverage   99.63%   99.63%             
=========================================
  Files        2337     2337             
  Lines      241178   242692   +1514     
  Branches     1098     1098             
=========================================
+ Hits       240294   241808   +1514     
  Misses        863      863             
  Partials       21       21             
Impacted Files Coverage Δ
src/modules/color/index.ts 99.79% <100.00%> (+0.05%) ⬆️
src/modules/datatype/index.ts 100.00% <100.00%> (ø)
src/modules/date/index.ts 99.79% <100.00%> (+0.05%) ⬆️
src/modules/finance/index.ts 100.00% <100.00%> (ø)
src/modules/git/index.ts 100.00% <100.00%> (ø)
src/modules/helpers/index.ts 98.98% <100.00%> (+0.11%) ⬆️
src/modules/image/index.ts 100.00% <100.00%> (ø)
src/modules/internet/index.ts 100.00% <100.00%> (ø)
src/modules/lorem/index.ts 100.00% <100.00%> (ø)
src/modules/number/index.ts 100.00% <100.00%> (ø)
... and 5 more

@xDivisionByZerox
Copy link
Member

@ST-DDT @xDivisionByZerox Before I move on copying over all the stuff from one place to another: is this going in the right direction, or do we need to re-phrase something? Also did I missed something? This is for now an example for the color module (the animal module didn't had options)

Yes, that would be the approach we discussed in yesterday's meeting. Altho, I would likely write a script that does the copy and past, so we could add it to our toolchain (pre-commit or something).

@Shinigami92
Copy link
Member Author

Altho, I would likely write a script that does the copy and past, so we could add it to our toolchain (pre-commit or something).

@xDivisionByZerox While copy and pasting these lines, I came to the exact same conclusion and would prefer these absolutely
The only sad thing about this: it's definitely out of my salary-class while working on faker 😆
I mean I don't have the 🧠 for doing such scripts or at least not the motivation

@ST-DDT
Copy link
Member

ST-DDT commented Dec 9, 2022

@Shinigami92 Maybe we can sit together in the next few days and work on a solution.

@Shinigami92 Shinigami92 marked this pull request as ready for review December 14, 2022 10:47
@Shinigami92 Shinigami92 requested a review from a team as a code owner December 14, 2022 10:47
@Shinigami92 Shinigami92 requested a review from a team December 14, 2022 10:47
ST-DDT
ST-DDT previously approved these changes Jan 5, 2023
docs: jsdoc for options
@Shinigami92
Copy link
Member Author

@ST-DDT @xDivisionByZerox @import-brain please review/approve this so we can omit further merge conflicts
it's currently for me a non-goal to automate stuff as this could be done in a separate PR if really needed/wanted

this PR just improved the actual DX for faker users and we can manually keep care of reviews in the future when someone adds options and also as well add docs to options if there are still missing ones that wasn't catched

@ST-DDT ST-DDT added the p: 1-normal Nothing urgent label Jan 17, 2023
Copy link
Member

@xDivisionByZerox xDivisionByZerox left a comment

Choose a reason for hiding this comment

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

That was way to much. Should have simply trusted you xD

@ST-DDT
Copy link
Member

ST-DDT commented Jan 17, 2023

That was way to much.

Yes

@Shinigami92
Copy link
Member Author

Should have simply trusted you xD

to be fair, I already reviewed what GitHub Co-Pilot wrote onto my screen 🙂

@xDivisionByZerox
Copy link
Member

The pipeline was canceled - any idea why? I would like to merge this ASAP due to the changes having global effects.

@Shinigami92
Copy link
Member Author

I would like to merge this ASAP due to the changes having global effects.

Okay I will merge right now 4h earlier then preferred

@Shinigami92 Shinigami92 merged commit 250fbb7 into next Jan 18, 2023
@Shinigami92 Shinigami92 deleted the 1586-jsdoc-for-options branch January 18, 2023 20:02
@xDivisionByZerox
Copy link
Member

I would like to merge this ASAP due to the changes having global effects.

Okay I will merge right now 4h earlier then preferred

That was not what I meant, but appreciated xD I was just wondering if the pipeline was actually failing or canceled on purpose.

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
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants