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

Add .jenkins file for new CI #232

Merged
merged 7 commits into from
Dec 3, 2018
Merged

Add .jenkins file for new CI #232

merged 7 commits into from
Dec 3, 2018

Conversation

marcoceppi
Copy link
Contributor

WIP

@marcoceppi marcoceppi added the WIP Work In Progress label Nov 29, 2018
@codecov
Copy link

codecov bot commented Nov 30, 2018

Codecov Report

Merging #232 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #232   +/-   ##
=======================================
  Coverage   87.76%   87.76%           
=======================================
  Files          46       46           
  Lines        1381     1381           
=======================================
  Hits         1212     1212           
  Misses        169      169

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6df9323...8a3868f. Read the comment docs.

@edaniszewski
Copy link
Contributor

After getting hung up on a bunch of really tiny things, I think this is pretty much good. I tested the 'publish edge image', 'draft github release', and 'publish release images' by temporarily setting the when block to this branch (and faking a tag by manually setting env variables). It seems like that should work, as long as jenkins triggers on the branches/tags which I think should be fine.

there is definitely room for improvement/simplification of things (e.g. if there is a jenkins plugin for github releasing, that would probably be better than the tooling we use now), but with this we should at least have parity with the circleci config.

there are two parts that are not quite all the way there:

  • one is not super important right now, but it is setting PR status at different stages (right now, this is only set in the linting stage as a test). Its not totally necessary, but it would make it easier to see what failed and maybe why. It could also remove the need to hook up to the external code coverage tool if we can just report coverage ourselves.
  • the other is integrating with codecov. if the above bit gets resolved, we probably won't need to use it anymore so this is moot. otherwise, we need to set the repo upload token. its not a hard thing to do, just need to do it; first I think it makes sense to decide whether to keep it or just report coverage ourselves if we can get that working

@edaniszewski edaniszewski removed the WIP Work In Progress label Dec 3, 2018
@edaniszewski
Copy link
Contributor

This should be ready for review.

This PR adds the Jenkins config which has feature parity with the CircleCI config. It also removes the Circle config. Project settings have been updated to no longer use Circle.

@marcoceppi
Copy link
Contributor Author

It looks like codecov isn't working just yet

      _____          _
     / ____|        | |
    | |     ___   __| | ___  ___ _____   __
    | |    / _ \ / _  |/ _ \/ __/ _ \ \ / /
    | |___| (_) | (_| |  __/ (_| (_) \ V /
     \_____\___/ \____|\___|\___\___/ \_/
                                    v2.0.15

==> Detecting CI provider
    Jenkins Detected
==> Preparing upload
Error: Missing repository upload token

Tip: See all example repositories: https://github.com/codecov?query=example
Support channels:
  Email:   hello@codecov.io
  IRC:     #codecov
  Gitter:  https://gitter.im/codecov/support
  Twitter: @codecov

Copy link
Contributor Author

@marcoceppi marcoceppi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things of point, but otherwise this look phenominal

@@ -17,6 +17,32 @@ pipeline {
steps {
sh 'make lint'
}
post {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will happen automatically, shouldn't need to call it out explicitly. There's a problem with our Jenkins after a plug-in update that isn't working

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking back now, I think this is fine - it's a bit verbose but it'll help show why pull requests fail

.jenkins Outdated
status: 'failure',
context: 'vio-build/lint',
description: 'Source code linting failed',
targetUrl: "${env.JOB_URL}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of ${env.JOB_URL} use ${env.RUN_DISPLAY_URL} which will take you to the Blue Ocean page and not the broke ass Jenkins legacy page

@edaniszewski
Copy link
Contributor

the tests that are failing here should be fixed in #233. Those changes will be merged into this branch, at which point this should be good to go into master.

@edaniszewski edaniszewski merged commit 0bb87b5 into master Dec 3, 2018
@edaniszewski edaniszewski deleted the jenkins-ci branch December 3, 2018 23:02
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.

None yet

2 participants