-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
src: add public API to get default node platform #17946
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
Conversation
|
Reference issue #17859 |
src/node.h
Outdated
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.
Poor name since it implies there is also a non-default platform. Just GetPlatform()?
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 was thinking this platform is created in node::Start so it seems to be the default one.
And since we also have CreatePlatform api, I don't want people to think they can get newly created platform with this.
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.
How about GetCurrentPlatform()?
src/node.cc
Outdated
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 you undo the whitespace changes?
|
In reference to #17859, I will say that it's still not entirely clear to my what you're doing that necessitates this. |
4204a26 to
a37abb5
Compare
a37abb5 to
dc1e228
Compare
|
@levimm could you elaborate what this is necessary for? |
|
Ping @levimm |
1 similar comment
|
Ping @levimm |
|
Sorry for the late response, @BridgeAR. |
|
I realize this doesn't help much to solve my problem. My real issue is PumpMessageLoop is removed from node and I have to include extra v8_libplatform dependency in order to call this method. I'll close this one. |
|
For posterity, bringing back PumpMessageLoop() is issue #17254. |
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
embedding