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(guide): add complex overwrites example #3207

Merged
merged 5 commits into from
Oct 21, 2024

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Oct 20, 2024

Fixes #1508


The PR adds an additional example to the complex objects creation documentation.

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent labels Oct 20, 2024
@ST-DDT ST-DDT added this to the vAnytime milestone Oct 20, 2024
@ST-DDT ST-DDT requested review from a team October 20, 2024 21:45
@ST-DDT ST-DDT self-assigned this Oct 20, 2024
Copy link

netlify bot commented Oct 20, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 85d8728
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/671663027fe5da0008e85937
😎 Deploy Preview https://deploy-preview-3207.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Oct 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.96%. Comparing base (f917f4f) to head (85d8728).
Report is 1 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff            @@
##             next    #3207    +/-   ##
========================================
  Coverage   99.95%   99.96%            
========================================
  Files        2798     2798            
  Lines      216718   216718            
  Branches      577      955   +378     
========================================
+ Hits       216631   216642    +11     
+ Misses         87       76    -11     

see 1 file with indirect coverage changes

@matthewmayer
Copy link
Contributor

Hmm this seems more like a "how to use typescript" than anything faker-specific.

@ST-DDT
Copy link
Member Author

ST-DDT commented Oct 21, 2024

The entire section is more about raising awareness of concepts you could use with faker.
The NodeJS-Section explains how you can use faker and the Create Complex Objects section expands on this with ways to structure your methods for complex objects.

A secondary goal is push users into this style of creating "faker" methods, so that they can more easily switch to standalone module functions (see the code examples in the alternatives section) if they would like to when we introduce them in v10.

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.

I'm fine with this, since there was an explicit call out from some community members for this type of documentation.

However, I agree with Matthew that this content seems more related to general programming practices rather than faker specifically. Every programmer should be able to identify these situations as the a requirement for creation design pattern.
Our documentation primarily focuses on the factory pattern. Is it necessary to prime consumers towards a specific programming style, when other approaches, such as the builder pattern, might be better suited for their needs? While it’s not inherently bad, it might limit the flexibility for users to choose the best method for their particular use case.

Furthermore, I fear that the inclusion of documentation that focuses on the developer interaction with faker instead of the usage of faker might set expectations for our community that we don't want to get involved in.

@matthewmayer
Copy link
Contributor

I'll grudgingly approve 😂

Sometimes you see these kind of worked examples in a kind of "cookbook" section. Could be worth considering in future.

@xDivisionByZerox xDivisionByZerox enabled auto-merge (squash) October 21, 2024 14:19
@xDivisionByZerox xDivisionByZerox merged commit 0d1616d into next Oct 21, 2024
21 checks passed
@xDivisionByZerox xDivisionByZerox deleted the docs/guide/usage/complex-overwrites branch October 21, 2024 14:23
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
None yet
Development

Successfully merging this pull request may close these issues.

Document how to write factory functions with value overwrites
4 participants