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

Rustfmt updates + add to Travis #60

Merged
merged 11 commits into from
Apr 2, 2018
Merged

Conversation

psivesely
Copy link
Contributor

I rebased this branch on my validation-2 (#59) already, and can rebase this branch again on the master of this repo if/when that PR is merged.

This PR updates formatting throughout the crate based on the latest Clippy version (as of a few days ago). I think the improvements are good.

I also added Clippy to Travis, and made a few changes that should make the Travis builds go faster, including use of the cache, and fast_finish, so we don't have to wait for the nightly build to finish to get the overall result, since that build is allowed to fail anyway.

@psivesely psivesely mentioned this pull request Apr 1, 2018
@romac
Copy link
Member

romac commented Apr 2, 2018

Argh, I did not mean to create a merge commit. Feel free to drop it when you rebase this PR against master.

Looks like rustfmt has made some improvements recently, so wanted to bring the
code up to date.
Items that match the `allow_failures` predicate (right now, just Rust nightly),
will still finish, but Travis won't wait for them to report a result if the
other builds have already finished.
@psivesely
Copy link
Contributor Author

Okay, rebased.

I noticed cargo-kcov is currently being run by every build, when it should be run by a single matrix that's allowed_failure. If you want to block on coverage regressions then the block should come from, e.g., using the CodeCov integration. But this is of course a completely different issue. Can make a follow-up PR to address if you'd like.

@romac
Copy link
Member

romac commented Apr 2, 2018

I noticed cargo-kcov is currently being run by every build, when it should be run by a single matrix that's allowed_failure.

Sounds good! You can do it in this PR if you want.

We don't want rustfmt to match `allow_failures` just because it needs to use
nightly, while we do want nightly to match `allow_failures`. Env vars provide a
solution.
Some of the dependencies we were installing were not listed on
https://github.com/SimonKagstrom/kcov/blob/master/INSTALL.md, and we were
missing one dependency that was listed there. When `sudo: true` Travis uses
Ubuntu Trusty.
kcov builds its own test executables.
@psivesely
Copy link
Contributor Author

Okay, everything is looking good and working how I expected. Going to wait for build job #192.5 to finish, then going to push the final commit. This will be an opportunity to confirm that Travis's cargo cacheing, and the cargo-update tool are working how we expect as well.

As noted in aeb3906 it is not necessary to
build the project before running kcov, but kcov does require a `Cargo.lock`
file, which can be generated with `cargo update`.
@psivesely
Copy link
Contributor Author

Travis runtime cut from 9m to 3m (or 4m for the kcov build we don't have to wait for because we're using fast_finish now) and everything seems to be working how it should. Think this one is ready for merge after your re-review.

@romac
Copy link
Member

romac commented Apr 2, 2018

Very nice! Thanks a lot!

@romac romac merged commit 12cefb1 into SpinResearch:master Apr 2, 2018
romac pushed a commit that referenced this pull request Aug 13, 2018
* Update rustfmt compliance

Looks like rustfmt has made some improvements recently, so wanted to bring the
code up to date.

* Add rustfmt to nightly item in Travis matrix

* Use Travis Cargo cache

* Allow fast_finish in Travis

Items that match the `allow_failures` predicate (right now, just Rust nightly),
will still finish, but Travis won't wait for them to report a result if the
other builds have already finished.

* Run kcov in a separate matrix build in Travis

* Rework allowed_failures logic

We don't want rustfmt to match `allow_failures` just because it needs to use
nightly, while we do want nightly to match `allow_failures`. Env vars provide a
solution.

* Add --all switch to rustfmt Travis

* Test building docs in Travis

* Use exact Ubuntu dependencies listed for kcov

Some of the dependencies we were installing were not listed on
https://github.com/SimonKagstrom/kcov/blob/master/INSTALL.md, and we were
missing one dependency that was listed there. When `sudo: true` Travis uses
Ubuntu Trusty.

* No need to build before running kcov

kcov builds its own test executables.

* Generate `Cargo.lock` w/ `cargo update` before running kcov

As noted in aeb3906 it is not necessary to
build the project before running kcov, but kcov does require a `Cargo.lock`
file, which can be generated with `cargo update`.
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