-
Notifications
You must be signed in to change notification settings - Fork 683
Update the API changes in mbed targets #1175
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
@@ -54,7 +54,7 @@ int js_entry (const char *source_p, const size_t source_size) | |||
} | |||
} | |||
} | |||
|
|||
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.
seems a trailing white space... is this intended?
#1177 is merged. docs/02.API-EXAMPLES.md is up to date, but docs/02.API-REFERENCE.md is not. Let me know, if you need any help with the update. |
@polaroi8d, please check #1201 too. |
Thanks @LaszloLango, I will check it. |
@@ -31,11 +31,10 @@ | |||
|
|||
#define DECLARE_HANDLER(NAME) \ | |||
static bool \ |
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.
static jerry_value_t \
Tested on FRDM-K64F, works fine. |
|
||
return 0; | ||
return ret_val; |
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.
why?
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.
Mixed with the DECLARE_HANDLER
return value.
Only style issues remained. |
LGTM |
port = (int)JS_VALUE_TO_NUMBER (&args_p[0]); | ||
value = (int)JS_VALUE_TO_NUMBER (&args_p[1]); | ||
port = (int) jerry_get_number_value (args[0]); | ||
value = (int) jerry_get_number_value (args[1]); |
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.
Is it ensured that the arguments are numbers?
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.
No, thanks for the hint.
if (args_cnt < 2) | ||
{ | ||
return false; | ||
ret_val = jerry_create_boolean (false); | ||
printf ("Error: invalid arguments number!\r\n"); |
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.
You should add a return after this.
Updated the PR, added returns if the execution false. |
return false; | ||
jerry_release_value (global_object_val); | ||
jerry_release_value (reg_function); | ||
jerry_release_value (set_result); |
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.
Isn't this a compiler error? set_result has not initialized yet.
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.
This is bad news if the compiler does not warn about these :(
c59ab2b
to
69b4f51
Compare
jerry_api_release_object (global_obj_p); | ||
jerry_value_t set_result = jerry_set_property (global_object_val, | ||
jerry_name, | ||
reg_function); |
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.
Indentation error.
LGTM after the style fix. |
0e5a2e2
to
155ec47
Compare
- Update the API changes in mbed targets - Build fix for mbed target after the build system patch. JerryScript-DCO-1.0-Signed-off-by: Levente Orban [email protected]
Updating the ports because the API of JerryScript has been changed recently.