-
Notifications
You must be signed in to change notification settings - Fork 686
Modified mbedk64f target #982
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
|
There are some problems with the renamed files, because if we rewrite their names, didn't find specific libaries, like cstddef.
|
|
LGTM. |
| #include <stdlib.h> | ||
| #include <stdio.h> | ||
|
|
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.
These two empty lines help to separate the include types, such as jerry API, system and target headers. Why did you remove them?
|
@zherczeg I guess. |
Delete DECLARE_HANDLER(print) & double newlines.
Set some separator to the include types for good differentiation.
Fix the { usage.
Check the err_obj_p & released.
JerryScript-DCO-1.0-Signed-off-by: Levente Orban orbanl@inf.u-szeged.hu
|
@polaroi8d I've taken a closer look at the 4 open mbed-related PRs (this one + #981, #986, #989). They are way too much clones of each other. Except for differences in style (whitespace, comments, order of includes), the I'd propose unifying the mbed-related targets to prevent duplications. I could imagine a structure like Since the k64f support is already in the code base, I propose to update this PR to lay the foundations of the new structure as well as update the code to latest jerry (please note that an external port implementation is needed as identified in #1032). Do have some discussion with @knightburton to make sure that the other PRs are aligned. |
|
@akiss77 there is a new PR #1019 where we already done, what you describe. |
|
@polaroi8d Ahm, working my way up from the bottom of the list, I did not get that far. My bad. However, if these four PRs are superceded by #1019 then could you please close these with an appropriate comment so they don't show up anymore as ones waiting for review? |
|
@akiss77 Of course. |
|
In this and some other PRs what we did, they are way too much clones of each other, so this is the reason why I'm closed this pull request, because we created a new PR ( #1019 ) where merged the mbed releated targets into one source. |
As the mbed is a cpp project from which several classes were used we need to have .cpp files.
JerryScript-DCO-1.0-Signed-off-by: Levente Orban orbanl@inf.u-szeged.hu