-
-
Notifications
You must be signed in to change notification settings - Fork 257
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
Split Travis build into stages #62
Conversation
Changes Unknown when pulling 677c2f8 on hawkw:eliza/build-stages into ** on carllerche:master**. |
Changes Unknown when pulling a359e99 on hawkw:eliza/build-stages into ** on carllerche:master**. |
Okay, so I have code coverage working finally using |
thanks to xd009642/tarpaulin@8e3fe3a, coverage may actually work now (fingers crossed!) |
Codecov Report
@@ Coverage Diff @@
## master #62 +/- ##
=========================================
Coverage ? 50.41%
=========================================
Files ? 44
Lines ? 4953
Branches ? 0
=========================================
Hits ? 2497
Misses ? 2456
Partials ? 0 Continue to review full report at Codecov.
|
Changes Unknown when pulling 4a7a8de on hawkw:eliza/build-stages into ** on carllerche:master**. |
Changes Unknown when pulling 940afe9 on hawkw:eliza/build-stages into ** on carllerche:master**. |
Looks like stuff is happening... What are the next steps? |
0241578
to
521340a
Compare
Okay @carllerche, I think this will be ready to merge after build #320 is done. I've disabled the codecov.io bot comment (since it's configured in YAML), but I haven't disabled the coveralls bot, since that has to be done in the repo's settings on Coveralls (which I don't have access to). |
Changes Unknown when pulling 521340a on hawkw:eliza/build-stages into ** on carllerche:master**. |
Looks good. We probably should stick w/ one option to limit noise. Could you remove coveralls? |
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, few questions
.ci/coverage.sh
Outdated
#!/bin/bash | ||
|
||
# this is in a separate script from `.travis.yml` so we can set this option. | ||
shopt -s extglob |
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.
always set -eu
unless it's really clear that this isn't necessary.
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.
this script is actually no longer used – I should rm it
.ci/coverage.sh
Outdated
cmake .. && | ||
make && | ||
make install DESTDIR=../tmp && | ||
cd ../.. |
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 use set -e
, don't need these long &&
chains
.ci/coverage.sh
Outdated
cd ../.. | ||
|
||
if [ $TRAVIS_RUST_VERSION = nightly ]; then | ||
./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern=/.cargo --coveralls-id=$TRAVIS_JOB_ID --verify target/kcov target/debug/*-*[!.]? && |
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.
be careful to quote "$TRAVIS_JOB_ID" so it can't be used to inject nefarious things ;)
(PSA shellcheck
is a great tool that will complain about this sort of thing
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.
can I brew install shellcheck
?
.ci/coverage.sh
Outdated
shopt -s extglob | ||
|
||
wget https://github.com/SimonKagstrom/kcov/archive/master.tar.gz && | ||
tar xzf master.tar.gz && |
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.
just so i have context...
will this all run on every commit? can we cache the install directory via travis so that we only need to build if the cache is gone? If so, this should probably be guarded by if [ -x tmp/kvoc ] ...
or something?
@carllerche / @olix0r, this should be ready to merge now? |
Yes! Thanks for putting in the work and getting this done. I'll merge and any further tweaks can happen separately. |
I merged in master, I guess it has to go through CI again! |
This PR rewrites our Travis config to use Travis' Build Stages feature. It also includes my PR with fixes to codecov.io coverage uploading (#61). I've rewritten the step for publishing RustDoc to GitHub Pages to use a Travis Deploy stage, removing the dependency on
travis-cargo
.Closes #59
Closes #61