Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

chakrashim,test: fix Promise API compatibility #537

Merged
merged 2 commits into from
May 12, 2018

Conversation

kfarnung
Copy link
Contributor

@kfarnung kfarnung commented May 11, 2018

Update the shim to use the new JsGetPromiseResult and
JsGetPromiseState APIs for the v8::Promise shim.

NOTE: the changes to ChakraCore are being reviewed separately chakra-core/ChakraCore#5131

NOTE: waiting for chakra-core/ChakraCore#5138

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@@ -547,6 +547,7 @@
};

utils.isPromise = function(obj) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this isn't referenced any more (right?) can it be removed completely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still use here: https://github.com/nodejs/node-chakracore/pull/537/files#diff-55320053aaa0abe61795d171a27dcbc8L403

I believe that code path is no longer used as well, but I didn't want to mess with that right now.

@kfarnung kfarnung force-pushed the promises branch 2 times, most recently from 5419507 to bd06de6 Compare May 11, 2018 21:33
@kfarnung
Copy link
Contributor Author

kfarnung commented May 11, 2018

PR-URL: nodejs#537
Reviewed-By: Jimmy Thomson <[email protected]>
Reviewed-By: Seth Brenith <[email protected]>
Update the shim to use the new `JsGetPromiseResult` and
`JsGetPromiseState` APIs for the `v8::Promise` shim.

PR-URL: nodejs#537
Reviewed-By: Jimmy Thomson <[email protected]>
Reviewed-By: Seth Brenith <[email protected]>
@kfarnung kfarnung merged commit b34f2f8 into nodejs:master May 12, 2018
@kfarnung kfarnung deleted the promises branch May 12, 2018 01:01
kfarnung added a commit to kfarnung/node-chakracore that referenced this pull request May 22, 2018
Update the shim to use the new `JsGetPromiseResult` and
`JsGetPromiseState` APIs for the `v8::Promise` shim.

PR-URL: nodejs#537
Reviewed-By: Jimmy Thomson <[email protected]>
Reviewed-By: Seth Brenith <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants