-
Notifications
You must be signed in to change notification settings - Fork 105
Re-enable nRF52840 lock example targets in travis #311
base: master
Are you sure you want to change the base?
Conversation
It's not clear why the .travis/build_nrf52840_lock_example.sh or .travis/prepare_nrf52840_lock_example.sh are in openweave-core rather than openweave-nrf52840-lock-example. |
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.
It's not clear why the .travis/build_nrf52840_lock_example.sh or .travis/prepare_nrf52840_lock_example.sh are in openweave-core rather than openweave-nrf52840-lock-example.
I realize those two files are now removed with this PR; however, it's not clear why the content is in or should be in this repo. |
67d3f84
to
5f026ae
Compare
@gerickson, the idea here is to make sure that changes going into openweave core don't break the lock example app. This target did exist earlier but was disabled by Jay in commit e509266. I just re-enabled it and cleaned up the scripts a little. |
Why aren't we doing that CI in openweave-nrf52840-lock-example? |
@gerickson We do. But openweave-nrf52840-lock-example doesn't build when a PR is submitted against openweave-core. In this case, building the lock example within openweave-core, using the submitted code from the PR, gives the author visibility into whether their change is going to break something on that platform. The ESP32 target works the same way. |
.travis/before_install.sh
Outdated
|
||
# Tool download links | ||
# | ||
export NORDIC_SDK_URL=https://www.nordicsemi.com/-/media/Software-and-other-downloads/SDKs/nRF5-SDK-for-Thread/nRF5-SDK-for-Thread-and-Zigbee/nRF5SDKforThreadandZigbeev300d310e71.zip |
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 are these values exported?
I believe we need to find a more scalable solution. The ESP32 target was a acceptable one-off; however, now that we are adding more demo / example integrations, it's time to find a better, more scalable approach. While it doesn't look like Travis CI has a way to trigger another project to build out of the box the way Atlassian Bamboo did/does, but maybe this hints at a path by which we can effect something similar: https://hiddentao.com/archives/2016/08/29/triggering-travis-ci-build-from-another-projects-build |
The linked example doesn't do what we need. In particular, we need the openweave-core build to fail I don't really understand the scale issue. Given that we need to maintain two related, but not identical build actions across each example--one that builds commits against the example project, and one that builds the example project for commits against openweave-code--ultimately I think the mount of support effort is going to be the same. Granted, the current state of the build/prepare scripts forces us to have two implementations of largely identical behavior. But we should be able to restructure the scripts such that the code can be shared between the two projects. With this done, we might be able to have the openweave-core build pull the build/prepare scripts from the example repos as part of the build process, such that there is only one master copy of the shared scripts. |
5f026ae
to
f9c707f
Compare
-- Enable building nRF52840 Lock Example on Linux and OSX -- Rather than installing dependencies for building lock example app in the before install travis script, call the travis prepare script from the lock example repo once it has been cloned. This makes sure that dependency installation for lock example is not duplicated in openweave travis scripts.
f9c707f
to
23456bc
Compare
I re-worked the scripts in openweave-core build to pull the prepare script from the example repo as part of the build process, such that there is only one master copy of the shared scripts. |
-- Source nrf_setup_vars.sh when building lock example app targets to bring in the env vars that were generated during the prepare script.
d33466d
to
e512e37
Compare
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.
Approved in concept. I'll let you and @jaylogue iterate on the best way, short-term, to eliminate copy-and-paste among the projects.
-- Download sdk and tools for the nrf52840 lock example target in the
before_install script rather than in the prepare script. This makes sure
that dependancy installation happens only once as we add more
applications in the future for the nrf52840 platform that use the
same dependencies.
-- Enable building nRF52840 Lock Example on Linux and OSX