-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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 initial constants module documentation #6505
Conversation
#### constants.RSA_PKCS1_PSS_PADDING | ||
#### constants.POINT_CONVERSION_COMPRESSED | ||
#### constants.POINT_CONVERSION_UNCOMPRESSED | ||
3### constants.POINT_CONVERSION_HYBRID |
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.
Markdown typo here :)
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.
Sigh ... fat fingers
|
||
### constants.X_OK | ||
|
||
Flag indicating that the file can be executed by the calling process. |
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’d add something like:
This flag is meant for use with [`fs.access()`][].
to (each of?) these, otherwise people might confuse them with O_RDONLY
etc.
[`fs.access()`]: fs.html#fs_fs_access_path_mode_callback
Cool, definitely thanks for the work you put into this! |
Of course. It's not very helpful for me to complain about there not being documentation for something unless I'm willing to help get the documentation put together, right? ;-) |
this is big enough I think it warrants direct ctc coversation |
@nodejs/ctc ... Any thoughts on this? |
Would a table not suite the plenty constants better? |
Possibly. Went with this initially because it's a closer fit to the Would a table not suite the plenty constants better? — |
Ah, you mean like the "events"-section. That sounds even better. Would be hugely favoring this. |
|
||
The following error constants are adapted directly from the POSIX `errno` | ||
standards. See http://man7.org/linux/man-pages/man3/errno.3.html for more | ||
details about specific error constants. |
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.
Link to the Errors documentation page maybe?
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.
Can, but those are far from complete. We should either fill those out there more or move that detail into here. How those are documented leaves much to be desired.
Is the object returned by |
@Fishrock123 No, only the constants themselves are all read-only properties, at least right now. |
|
||
### constants.ECHILD | ||
|
||
Indicates that there are no child processes |
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.
Missing period at the end of the line.
@cjihrig ... Thanks for the edits, I have yet to go through and do a proofread through this. Before I got too far into it tho, we need to decide if we even want to document these in this way or take a different approach. I wouldn't want you to spend too much time doing a proofread if we ultimately decide not to do this. |
Closing in favor of #6534 |
Checklist
Affected core subsystem(s)
doc (constants)
Description of change
This is a work in progress
Begin documentation of the constants module.
/cc @nodejs/documentation @addaleax @nodejs/crypto
Note to @nodejs/crypto: there appear to be several of the openssl related constants that are not recommended for use. I'd recommend that we indicate those and possible even do a hard deprecation for those at some point (but not in this PR)
Refs: #6492, #6494