use industrial_ci instead of custom CI

ros-naoqi / naoqi_driver

c++ bridge based on libqi

Apache License 2.0

54 stars 93 forks source link

use industrial_ci instead of custom CI #128

Closed mikaelarguedas closed 4 years ago

mikaelarguedas commented 4 years ago

Signed-off-by: Mikael Arguedas mikael.arguedas@gmail.com

WIP: experiment with industrial_ci to avoid custom logic in travis.yml

Pros:

start from scratch image so not rely on packages in desktop but actually checking that dependencies are declared properly
no custom logic, rely on vastly adopted tool instead
this runs the installation phase as well
allow finer grain control, e.g. here enabling melodic builds but allowing it to fail until it's ready for prime time

Cons:

takes a bit longer
runs on all branches (thats actually a pro for me but it's debatable)
send email notifications on success (need to be configured)

mbusy commented 4 years ago

This seems pretty cool, and the Travis logs are much more readable with this method. I'm guessing that the URL of the CI badges in the README will still be valid ?

mikaelarguedas commented 4 years ago

I'm guessing that the URL of the CI badges in the README will still be valid ?

Yeah I believe they will work without needing a change

mikaelarguedas commented 4 years ago

This has been rebased and conflicts resolved. Now ready for review

Regarding the cons from above:

takes a bit longer

With ccache enabled it's actually roughly the same time. More time installing dependencies so if the repos are slow job takes quite longer (e.g. indigo jobs fetching EOL packages take ~twice longer than the active ones like kinetic)

runs on all branches (thats actually a pro for me but it's debatable)

Seems ok to me, but I can configure it otherwise if it is preferred

send email notifications on success (need to be configured)

This is actually using travis default policy:

send emails on failure
send email on success only if the status changes (e.g. it was broken and is now fixed) Which seems reasonable. I can also change this if preferred just let me know.

mbusy commented 4 years ago

Only question before merging the PR: the email notifications are sent for event that only occur on the master branch, or all branches ?

mikaelarguedas commented 4 years ago

It'll be on any branch of this repo.

This allows you to test your branch before opening a PR. We can configure it to send email only for jobs on the master branch if preferred

mbusy commented 4 years ago

The fact that the CI runs on all branches is good, but yes, for the emails I'd rather receive notifications only for jobs on master

mikaelarguedas commented 4 years ago

The fact that the CI runs on all branches is good, but yes, for the emails I'd rather receive notifications only for jobs on master

Fair enough https://github.com/ros-naoqi/naoqi_driver/pull/128/commits/32ee4edbdec61f17070e0b7bb09211496c80193e

mbusy commented 4 years ago

Nice, merging