-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Conversation
This seems to be all the possible results for Can somebody confirm? |
ping @misterdjules @jasnell |
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 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?
|
looks fine to me but let's ask @trevnorris and @cjihrig to confirm |
@misterdjules ... ping again... can I ask for you to take a look at this? |
Wrap the lines at 80 chars (except for links). But otherwise LGTM. |
Here's the final table I'd like to include in docs
|
@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! |
I would just use a simple list.
|
|
LGTM! |
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
@jasnell Done. |
Yes. target master and we'll cherry pick to v3.x as appropriate. |
I'm running this through the CI now to get it landed. |
@jasnell Done :) |
Specifies origin and includes a list of possible values PR-URL: #25777 Reviewed-By: James M Snell <[email protected]>
Landed in 4a91fa1 |
Specifies origin and includes a list of possible values PR-URL: nodejs#25777 Reviewed-By: James M Snell <[email protected]>
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