-
Notifications
You must be signed in to change notification settings - Fork 441
Update JerryScript submodule to post 2.3 version #1949
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
Update JerryScript submodule to post 2.3 version #1949
Conversation
cmake/iotjs.cmake
Outdated
| message(STATUS "JERRY_MEM_STATS ${FEATURE_MEM_STATS}") | ||
| message(STATUS "JERRY_PROFILE ${FEATURE_PROFILE}") | ||
| message(STATUS "JERRY_DEBUGGER ${JERRY_DEBUGGER}") | ||
| message(STATUS "JERRY_HEAP_SIZE_KB ${JERRY_GLOBAL_HEAP_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.
Is this intentionally inconsistent? JERRY_HEAP_SIZE_KB vs JERRY_GLOBAL_HEAP_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.
At the moment I did not want to break the defines/options provided by IoT.js, but I think the name(s) should be normalized in a later change.
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.
Hm, what I mean is that the name that is printed is not the actual name of the variable that's printed. Shouldn't this be message(STATUS "JERRY_GLOBAL_HEAP_SIZE ${JERRY_GLOBAL_HEAP_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.
Ahh sorry (was confused by the macro names), true . The original one was also misnamed, but it makes sense to use it the way you mentioned.
tools/build.py
Outdated
| help='Enable JerryScript heap statistics') | ||
| jerry_group.add_argument('--jerry-profile', | ||
| metavar='FILE', action='store', default='es5.1', | ||
| metavar='FILE', action='store', default='es.next', |
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.
Open question: shall the bump to JerryScript 2.3 also include a bump in the supported ES version?
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.
Ohh. I've missed this. I've just wanted to change the comment below. Will update this asap to keep it at es5.1 for now.
78c58b8 to
74bb546
Compare
Based on: jerryscript-project#1933 Original author IoT.js-DCO-1.0-Signed-off-by: bence gabor kis [email protected] IoT.js-DCO-1.0-Signed-off-by: Peter Gal [email protected]
74bb546 to
7d18258
Compare
|
@akosthekiss thanks for the input I've updated the PR. |
akosthekiss
left a comment
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.
LGTM
zherczeg
left a comment
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.
LGTM
|
For the record this change fixes this vulnerability: |
Based on: #1933
Original author IoT.js-DCO-1.0-Signed-off-by: bence gabor kis [email protected]