Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

hertzg
Copy link

@hertzg hertzg commented Jul 28, 2015

Clarifies that os.platform() value is based on gyp, differences in values for Mac OS X, Windows and Solaris and provides some possible values.

Resolves #25769

/cc @jasnell

@hertzg
Copy link
Author

hertzg commented Jul 29, 2015

After some more investigation

System os.platform()
Android android
Windows/Cygwin win32
Mac OS X darwin
FreeBSD freebsd
OpenBSD openbsd
IBM AIX aix
Solaris sunos
Linux & Others linux

This seems to be all the possible results for os.platform()

Can somebody confirm?

@hertzg
Copy link
Author

hertzg commented Aug 3, 2015

ping @misterdjules @jasnell

@jasnell
Copy link
Member

jasnell commented Aug 4, 2015

LGTM. @hertzg , thanks for the ping, the notification on this got buried in my notifications pile. I'll assign this to myself and will hopefully be able to get it landed by early next week.

@jasnell jasnell self-assigned this Aug 4, 2015
@hertzg
Copy link
Author

hertzg commented Aug 4, 2015

@jasnell I'd like to confirm the table I've provided and would like to include it in the documentation. Will it be any problem to put this table in docs?

System os.platform()
Android android
Windows/Cygwin win32
Mac OS X darwin
FreeBSD freebsd
OpenBSD openbsd
IBM AIX aix
Solaris sunos
Linux & Others linux

@jasnell
Copy link
Member

jasnell commented Aug 4, 2015

looks fine to me but let's ask @trevnorris and @cjihrig to confirm

@jasnell jasnell added the doc label Aug 6, 2015
@hertzg
Copy link
Author

hertzg commented Aug 9, 2015

@jasnell
Copy link
Member

jasnell commented Aug 14, 2015

@misterdjules ... ping again... can I ask for you to take a look at this?

@jasnell jasnell added the v0.12 label Aug 14, 2015
@trevnorris
Copy link

Wrap the lines at 80 chars (except for links). But otherwise LGTM.

@hertzg
Copy link
Author

hertzg commented Aug 17, 2015

@trevnorris

I'd like to confirm the table I've provided and would like to include it in the documentation. Will it be any problem to put this table in docs?

Here's the final table I'd like to include in docs

System os.platform()
Android android
Windows/Cygwin win32
Mac OS X darwin
FreeBSD freebsd
OpenBSD openbsd
IBM AIX aix
Solaris sunos
Linux & Others linux

@jasnell
Copy link
Member

jasnell commented Aug 17, 2015

@hertzg ... the table looks good. If you can update the commit to use the table and make sure there are no line wrapping or whitespace issues, then we'll get it landed! Thank you for being patient with us!

@hertzg
Copy link
Author

hertzg commented Aug 17, 2015

Current Markdown renderer does not seem to support Tables. Any Ideas?

screen shot 2015-08-18 at 3 33 25 am

@jasnell
Copy link
Member

jasnell commented Aug 17, 2015

I would just use a simple list.

 * Android: `android`
 * Windows/Cygwin: `win32`
(and so on)

@hertzg
Copy link
Author

hertzg commented Aug 17, 2015

screen shot 2015-08-18 at 3 47 13 am
@jasnell Done 👍

@jasnell
Copy link
Member

jasnell commented Aug 19, 2015

LGTM!

@jasnell
Copy link
Member

jasnell commented Aug 19, 2015

Can I ask you to squash the two commits down into a single commit and then I'll get it landed. Btw, you may also want to check to see if a similar change needs to be landed in nodejs/node master.

Specifies origin and includes a list of possible values
@hertzg
Copy link
Author

hertzg commented Aug 19, 2015

@jasnell Done.
Should I branch off master and target master in case of nodejs/node?

@jasnell
Copy link
Member

jasnell commented Aug 19, 2015

Yes. target master and we'll cherry pick to v3.x as appropriate.

@jasnell
Copy link
Member

jasnell commented Aug 19, 2015

I'm running this through the CI now to get it landed.

@hertzg
Copy link
Author

hertzg commented Aug 19, 2015

@jasnell Done :)

jasnell pushed a commit that referenced this pull request Aug 19, 2015
Specifies origin and includes a list of possible values

PR-URL: #25777
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Aug 19, 2015

Landed in 4a91fa1

jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Specifies origin and includes a list of possible values

PR-URL: nodejs#25777
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants