Update ChakraCore samples to latest JSRT #48
Conversation
|
/cc @liminzhu |
|
Fixes #47 |
|
Thanks for the Quick fix on the bug, and the review involves compile bugs and notes involving Ubuntu Linux 16.04 LTS. |
|
@obastemur why only fail_check a portion of the API calls? |
|
Why change the output of the Hello World example from |
There was a problem hiding this comment.
@jcoffland, @liminzhu, and @obastemur, I believe a code review is a good idea... Here is some notes.
| @@ -16,32 +30,40 @@ int main() | |||
| JsValueRef result; | |||
| unsigned currentSourceContext = 0; | |||
|
|
|||
| // Your script; try replace hello-world with something else | |||
| const char* script = "(()=>{return \'Hello world!\';})()"; | |||
| const char* script = "(()=>{return \'SUCCESS\';})()"; | |||
There was a problem hiding this comment.
I Believe it would be best to avoid changing the output strings for the samples whenever possible. This is a hello world example, so printing "Hello World" is the expected output.
| @@ -4,8 +4,22 @@ | |||
| //------------------------------------------------------------------------------------------------------- | |||
|
|
|||
| #include "ChakraCore.h" | |||
| #include <stdlib.h> | |||
There was a problem hiding this comment.
Might want to also include stddef.h, This is because nullptr is undefined on Ubuntu Linux 16.04 LTS without this header.
There was a problem hiding this comment.
@kphillisjr we don't need to include stddef.h explicitly. Feel free to open an issue if you experience a problem. (and please add all the steps to reproduce)
There was a problem hiding this comment.
The reason I ran into the issue is that I changed the standard compile flag. The examples currently use -std=c++0x, and I was compiling against the flag -std=c++11.
There was a problem hiding this comment.
makes sense. We don't force devs to use c++11 though. We just use it internally.
|
Cool we have so many people on board 👍 |
@jcoffland That comes straight from our native tests.
Good idea to show to devs each and every single method may return an error value there. |
|
@kphillisjr @liminzhu @jcoffland Thanks for the review |
No description provided.