Rename the jerry_value_has_error_flag function. #2290
Conversation
zherczeg
left a comment
zherczeg
left a comment
There was a problem hiding this comment.
Just a small changes. But lets wait a bit for others opinion.
docs/02.API-REFERENCE.md
Outdated
| - `value` - api value | ||
| - return value | ||
| - true, if the given `jerry_value_t` has the error flag set | ||
| - true, if the given `jerry_value_t` has error value. |
There was a problem hiding this comment.
has error value. -> is an error value
| * Error flag manipulation functions. | ||
| */ | ||
| bool jerry_value_has_error_flag (const jerry_value_t value); | ||
| bool jerry_value_is_error (const jerry_value_t value); |
There was a problem hiding this comment.
I think this could be moved after bool jerry_value_is_undefined (const jerry_value_t value);.
There was a problem hiding this comment.
The error flag related functions are grouped together. See the comment above: "Error flag manipulation functions."
I'd keep this separate group, but the comment should be updated
There was a problem hiding this comment.
It was good for a flag manipulation functions, but error is a value now. Then we should either grouping others (e.g string, number) or place these to other value management groups. That is my thoughts though.
There was a problem hiding this comment.
I don't have strong preferences. If we move it in the header, then the implementation must be moved too. Please be careful and keep to the same function order in the .h and .c files.
docs/02.API-REFERENCE.md
Outdated
| **Summary** | ||
|
|
||
| Returns whether the given `jerry_value_t` has the error flag set. | ||
| Returns whether the given `jerry_value_t` has error value. |
| * Error flag manipulation functions. | ||
| */ | ||
| bool jerry_value_has_error_flag (const jerry_value_t value); | ||
| bool jerry_value_is_error (const jerry_value_t value); |
There was a problem hiding this comment.
The error flag related functions are grouped together. See the comment above: "Error flag manipulation functions."
I'd keep this separate group, but the comment should be updated
imiklos
force-pushed
the
jerry_value_has_error_flag
branch
from
6f9a1bf to
3228257
Compare
|
@zherczeg @LaszloLango I updated the PR, made the changes you mentioned. |
|
Does the documentation follows the function declaration last? Or it has a different order? |
|
I did not change the order in the documentation. It is in the same order as before. Should I need to change it in the documentation too? |
|
@imiklos yes please |
imiklos
force-pushed
the
jerry_value_has_error_flag
branch
from
3228257 to
ed76735
Compare
|
I moved the function in the documentation to keep the same order, and also moved the in the #2291 PR. |
| bool jerry_value_is_promise (const jerry_value_t value); | ||
| bool jerry_value_is_string (const jerry_value_t value); | ||
| bool jerry_value_is_undefined (const jerry_value_t value); | ||
| bool jerry_value_is_error (const jerry_value_t value); |
There was a problem hiding this comment.
It would be better to keep these in sort. Please move it before ´jerry_value_is_function´. Or am I missing something?
Rename the function to represent it's real functionality. JerryScript-DCO-1.0-Signed-off-by: Istvan Miklos imiklos2@inf.u-szeged.hu
imiklos
force-pushed
the
jerry_value_has_error_flag
branch
from
ed76735 to
3db0602
Compare
3 participants
Rename the function to represent it's real functionality.
JerryScript-DCO-1.0-Signed-off-by: Istvan Miklos imiklos2@inf.u-szeged.hu