-
Notifications
You must be signed in to change notification settings - Fork 30k
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
doc: add annotation to writeFile data
as Object
#39167
doc: add annotation to writeFile data
as Object
#39167
Conversation
There was a problem hiding this 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 using *
is a great idea, I don't think we're using this anywhere else, and I personally find it confusing. You could simply put the info in the body of the docs, like this:
Line 695 in f4d0a6a
A `TypeError` will be thrown if `size` is not a number. |
Consolidating the various
Oh 😅
@mscdex @aduh95 I included the footnote marker instead of duplicating the wording everywhere so it could be consolidated (since it probably has a single implementation source under the hood—meaning they would all change together). Putting myself in the user's shoes, I figure if I saw Regarding the In terms of which marker to use, my preference would be superscript numbers because it's robust (supporting an unlimited number of them, whereas symbols like
|
IMO I don't think that's a problem. There is a lot of duplicated wording in various sections of the node docs. In general I think it's no different than clarifying anything else about the signature or behavior of various APIs. The end user will need to be reading the function description anyway. If we're going to be changing how we convey certain types of information in the node API documentation, we should probably do so on a wider scale with a solid plan. For now though I think it's enough to just add the missing wording where necessary, even if it's duplicated. |
That is not true for the example I gave above. But yes, I agree: if there is no precedent for footnotes, best not to start randomly now. I'll update to duplicating the wording and leave the DRYing for later. |
010ea71
to
941010d
Compare
doc/api/fs.md
Outdated
@@ -5094,6 +5105,9 @@ changes: | |||
* `encoding` {string} | |||
* Returns: {number} The number of bytes written. | |||
|
|||
If `string` is a plain object, like all variations of `writeFile`, it must have |
There was a problem hiding this 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 if this is applicable, but Object
is listed in the types, so I guessed yes. Wanted to specifically call out just in case.
ccbdccf
to
493a748
Compare
@aduh95 🙏 I don't have access to merge PRs yet (not until after my first PR lands, if I recall correctly). Could you assist? |
Landed in 9257372...6a4b4ce |
Fixes: #39152 PR-URL: #39167 Reviewed-By: Antoine du Hamel <[email protected]>
Fixes: #39152 PR-URL: #39167 Reviewed-By: Antoine du Hamel <[email protected]>
Fixes: #39152 PR-URL: #39167 Reviewed-By: Antoine du Hamel <[email protected]>
Fixes: #39152