-
Notifications
You must be signed in to change notification settings - Fork 682
Add Travis CI jobs for build testing several targets #2102
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
Add Travis CI jobs for build testing several targets #2102
Conversation
Hitherto, code under the `targets` directory was not tested and so its maintenance was sometimes speculative. This commit adds build testing for several targets to prevent them from bit rotting. Targets covered by this commit are: ESP8266, Mbed, Mbed OS 5, NuttX, RIOT, Tizen RT, and Zephyr. Some issues were revealed and fixed: - ESP8266: added missing include for `uint32_t` typedef. - Tizen RT: replaced missing `str_to_uint` with `strtol`. JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
ec80980
to
caebd10
Compare
## Targets for building ESP8266 with JerryScript. | ||
|
||
# Build the firmware (ESP8266 with JerryScript). | ||
script: |
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.
I'd add install
target as a dependency of this target to prevent the parallel "build" of script
and install
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.
If you agree in my opinion, then please do it in the other new makefiles too.
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.
That never happens (I mean the parallel execution of script and install). If all
is called as the default target of make, then it explicitly lists the order of execution by invoking sub-makes in a specific order (install, script). While .travis.yml
calls make twice, first with install
target in the install
stage and then with script
target in script stage. (The make targets intentionally reflect the names of the Travis CI job stage names to highlight which target is called in which stage.) If install
was a dependency of script
, install
would run again in script stage.
See:
- env: JOBNAME="ESP8266 Build Test"
cache: ccache
install: make -f ./targets/esp8266/Makefile.travis install
script: make -f ./targets/esp8266/Makefile.travis script
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.
I see now, why you did it in this way. It is good, leave as is.
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.
I like this PR, LGTM
Issues found during job scripting:
This PR does NOT want to deal with the above issue but they are left for target maintainers for consideration. |
@akosthekiss Please open an issue for them. |
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
Hitherto, code under the
targets
directory was not tested and soits maintenance was sometimes speculative. This commit adds build
testing for several targets to prevent them from bit rotting.
Targets covered by this commit are: ESP8266, Mbed, Mbed OS 5,
NuttX, RIOT, Tizen RT, and Zephyr.
Some issues were revealed and fixed:
uint32_t
typedef.str_to_uint
withstrtol
.JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu