Skip to content
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

use industrial_ci instead of custom CI #128

Merged
merged 2 commits into from
Dec 18, 2019
Merged

use industrial_ci instead of custom CI #128

merged 2 commits into from
Dec 18, 2019

Conversation

mikaelarguedas
Copy link
Member

@mikaelarguedas mikaelarguedas commented Dec 12, 2019

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
Copy link
Member

mbusy commented Dec 16, 2019

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
Copy link
Member Author

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 mikaelarguedas marked this pull request as ready for review December 17, 2019 23:59
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@mikaelarguedas
Copy link
Member Author

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
Copy link
Member

mbusy commented Dec 18, 2019

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

@mikaelarguedas
Copy link
Member Author

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
Copy link
Member

mbusy commented Dec 18, 2019

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

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@mikaelarguedas
Copy link
Member Author

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 32ee4ed

@mbusy
Copy link
Member

mbusy commented Dec 18, 2019

Nice, merging

@mbusy mbusy merged commit 208c96e into master Dec 18, 2019
@mbusy mbusy deleted the use_ici branch December 18, 2019 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants