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

Improve documentation of return values #1599

Closed
Shinigami92 opened this issue Nov 24, 2022 · 4 comments
Closed

Improve documentation of return values #1599

Shinigami92 opened this issue Nov 24, 2022 · 4 comments
Labels
c: docs Improvements or additions to documentation m: date Something is referring to the date module p: 1-normal Nothing urgent wontfix This will not be worked on
Milestone

Comments

@Shinigami92
Copy link
Member

As a user it was not instantly clear to me that the return value of a method like faker.date.past() returns a Date and not a string by just reading the docs @example tag.


Proposal:

Change return value comments

- // '2022-02-05T09:55:39.216Z'
+ // Date('2022-02-05T09:55:39.216Z')

Reason: This is actually executable and will indicate clearly that is not just a string

Also change the description to mention {@link Date} instance


Attention: We need to update also our documentation generator scripts

@Shinigami92 Shinigami92 added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug m: date Something is referring to the date module labels Nov 24, 2022
@xDivisionByZerox
Copy link
Member

It might be interesting that JS itself prints dates by omitting the quotes around the "date string".

This would look like this:
faker.date.past(); // 2022-02-05T09:55:39.216Z

Since we try to comply with other JS standards as well, I'd like to at least discuss this style.

@Shinigami92
Copy link
Member Author

I also found that out (accidentally) a few weeks ago
I think we should do that 👍

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jul 15, 2023

So, do we still need to discuss this? If we do id add the needs: discussion label. If we want to go with the JS Date format, I'd add the good first issue label.

@xDivisionByZerox xDivisionByZerox added the s: needs decision Needs team/maintainer decision label Aug 7, 2024
@xDivisionByZerox
Copy link
Member

Team Decision

We do not see the advantage of this change right now.
TypeScript is already giving sufficent feedback about the return type of date functions. Additionally, when looking at the examples in the documentation, the return type is mentioned directly above.

@xDivisionByZerox xDivisionByZerox closed this as not planned Won't fix, can't repro, duplicate, stale Oct 3, 2024
@xDivisionByZerox xDivisionByZerox added wontfix This will not be worked on and removed s: needs decision Needs team/maintainer decision s: accepted Accepted feature / Confirmed bug labels Oct 3, 2024
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: date Something is referring to the date module p: 1-normal Nothing urgent wontfix This will not be worked on
Projects
No open projects
Status: Todo
Development

No branches or pull requests

3 participants