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.

Sign up for GitHub

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

Jump to bottom

doc: Update docs for os.platform() #2446

Closed
wants to merge 1 commit into from
Closed

doc: Update docs for os.platform() #2446

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions doc/api/os.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,26 @@ on OS X and `'Windows_NT'` on Windows.

## os.platform()

Returns the operating system platform. Possible values are `'darwin'`,
`'freebsd'`, `'linux'`, `'sunos'` or `'win32'`. Returns the value of
Returns the operating system platform. Returns the value of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this change, except process.platform should be a link.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

`process.platform`.

Its value is based on [`OS` constant of `gyp`](https://chromium.googlesource.com/external/gyp/+/HEAD/docs/InputFormatReference.md#Predefined-Variables)
, but with exceptions in case of Mac OS X, Windows and Solaris.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reference isn't useful, please remove. Its not useful because following the link doesn't actually lead to a list of platform values, and of the 3 example values, two are wrong.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no defined list of values. gyp itself states the following

But other values may be encountered and this list should not be considered exhaustive.

Its based on pythons sys.platform value with some exceptions and those exceptions are specified. But it also can be specified manually.

There's also a list of possible values returned by the function bellow as a list .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node's platform list is known even if gyp's is infinitely variable (at least the official ones, but if someone ports to a new platform and docs don't describe it at nodejs.org, that's not a problem).


On Windows its value is always `"win32"`, on Mac OS X it's `"darwin"` and on
Solaris it's `"sunos"`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three values should be listed in the list below, not seperated out.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Windows its value is always "win32", on Mac OS X it's "darwin" and on
Solaris it's "sunos".

The windows exception shows that its always win32 on all architectures of windows.
The Mac OS X and Solaris examples contrast the fact that the value of OS constant from gyp is not the same as the one returned from this function.

os gyp os.platform
Windows win win32
Mac mac darwin
Solaris solaris sunos


Here's a list of possible values for some platforms:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,


* Android: `"android"`
* Windows/Cygwin: `"win32"`
* Mac OS X: `"darwin"`
* FreeBSD: `"freebsd"`
* OpenBSD: `"openbsd"`
* IBM AIX: `"aix"`
* Solaris: `"sunos"`
* Linux & Others: `"linux"`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire list should be moved to the docs for process.platform.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that's a good idea.

## os.arch()

Returns the operating system CPU architecture. Possible values are `'x64'`,
Expand Down