doc: Update docs for os.platform() #2446
Conversation
brendanashworth
added
doc
| , but with exceptions in case of Mac OS X, Windows and Solaris. | ||
|
|
||
| On Windows its value is always `"win32"`, on Mac OS X its `"darwin"` and on | ||
| Solaris its `"sunos"`. |
There was a problem hiding this comment.
... on Mac OS X it is (or it's) ... on Solaris it is ...
Specifies origin and includes a list of possible values
|
@targos Fixed all, thanks |
|
LGTM |
|
Why do we even have Regarding this PR: I feel the changes are a bit too verbose, and I'm not sure the background on how this variable is obtained is really relevant to the user. Also, please update |
| On Windows its value is always `"win32"`, on Mac OS X it's `"darwin"` and on | ||
| Solaris it's `"sunos"`. | ||
|
|
||
| Here's a list of possible values for some platforms: |
There was a problem hiding this comment.
As I said, this feels to verbose. How about changing this whole part to:
Returns the value of `process.platform`, Possible values are:
Also, please linkify process.platform.
There was a problem hiding this comment.
As its values is assigned on build and not during runtime. I believe its required to show the origin of it.
Totally agree on linking,
|
I agree with @silverwind, its too verbose, has information that's not useful, and also makes the docs worse in a way, because it causes the os.platform and process.platform docs to diverge. Better to simplify the os.platform() docs, link to process.platform, and then enumerate more platform values there. And about "Linux & Others"... what other non-linux platforms are identified as "linux"? |
|
👍 to putting platform map in |
|
@silverwind @sam-github This was discussed firstly on nodejs/node-v0.x-archive#25769 |
|
@hertzg you replied to all comments, but didn't push new commits to address the comments, I've not much to offer until then. |
|
@hertzg ... ping. did you see @sam-github 's last comment (#2446 (comment)) |
|
Closing because of inactivity. |
|
That's unfortunate. This was a good change, just needed to make sure the details were correct. Hopefully someone will pick this up again. |
- made the copy changes @sam-github suggested - updated the documentation link as the original was broken references nodejs#2446
* references closed nodejs#2446 * updates broken documentation link * fixes copy based on comments from @sam-github
Specifies origin and includes a list of possible values
Ref: nodejs/node-v0.x-archive#25777
/ping @jasnell