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

bug: Do not run clippy on beta toolchain #27

Merged
merged 3 commits into from Nov 14, 2018

Conversation

Projects
None yet
2 participants
@SeanPrashad
Contributor

SeanPrashad commented Nov 10, 2018

Fixes #25: Do not run clippy on beta toolchain

As observed in #24, we should only invoke Clippy for the Stable and Nightly build channels on Travis.

Unfortunately, versions of Clippy vary across channels. With Beta deriving from Nightly (which is allowed to fail), we'll just disable Clippy for Beta to still preview new releases.

@SeanPrashad

This comment has been minimized.

Contributor

SeanPrashad commented Nov 10, 2018

@0xazure Something is funky - Travis didn't kick off when I opened this PR 😕 I'm silly - https://travis-ci.org/0xazure/supernova/requests

@SeanPrashad SeanPrashad force-pushed the SeanPrashad:issue-25 branch 12 times, most recently from 716dc39 to 3e97481 Nov 10, 2018

Include condition on when to invoke Clippy
Clippy will now only run on Stable and Nightly release channels of Rust
as it's versions are inconsistent between channels.

@SeanPrashad SeanPrashad force-pushed the SeanPrashad:issue-25 branch from 3e97481 to d0c1b1a Nov 10, 2018

@SeanPrashad SeanPrashad changed the title from [WIP] bug: Do not run clippy on beta toolchain to bug: Do not run clippy on beta toolchain Nov 10, 2018

@SeanPrashad

This comment has been minimized.

Contributor

SeanPrashad commented Nov 10, 2018

This is ready for review @0xazure!

I've updated our .travis.yml file with a condition to only install and invoke clippy for the stable and nightly builds.

Examples of clippy being installed for stable can be found here, nightly here and beta here. Note that beta does not have any messages of installing Clippy, whilst the other two do:

Beta:

beta_builds

Stable:

stable_builds

Passing build (with updated .travis.yml)

Failing build (to showcase beta passes but stable and nightly doesn't)

@SeanPrashad SeanPrashad force-pushed the SeanPrashad:issue-25 branch from 50ceb82 to d0c1b1a Nov 10, 2018

@SeanPrashad

Some minor things to fix up

Show resolved Hide resolved .travis.yml
Show resolved Hide resolved .travis.yml
@0xazure

This comment has been minimized.

Owner

0xazure commented Nov 14, 2018

Hey @SeanPrashad this is looking really great, and we're green on the builds!

I know you just added a commit to reverse the release channel logic, but I think it would be better to be explicit about which channels we are using rather than filter by the ones we aren't.

Also, I'm looking at the implementation and correct me if I'm wrong but that looks like unix shell conditionals? I'm actually surprised that works correctly on the Windows builds, though a closer reading of the release announcement shows that builds run in a git bash shell, to maintain consistency with other bash-based environments.

I turned up some docs on Travis conditionals, do you know what their relation is to e.g. before_script? Are they strictly for jobs, or are they something we can use here too?

@SeanPrashad

This comment has been minimized.

Contributor

SeanPrashad commented Nov 14, 2018

I tried to play around with conditionals in Travis but I wasn't able to get the syntax working - see this build (even using the Or keyword kept complaining about syntax).

I ended up searching GitHub for "if [[TRAVIS_RUST_VERSION" as TRAVIS_RUST_VERSION provides the current build channel. I managed to find this existing example.

Upon digging around some more in Travis docs, there was another example that used if/then to install some packages with homebrew:

image

I'm willing to spend some more time to figure out how to incorporate Travis conditionals to make the file cleaner if you'd prefer that - let me know!

@0xazure

This comment has been minimized.

Owner

0xazure commented Nov 14, 2018

I've seen the bash-style conditionals in other projects' Travis configs as well, and thanks for linking to those other examples you turned up especially the one for the official docs.

This works as-is and Windows supports bashisms because of Travis' infrastructure, so I'm going to go ahead and merge this :shipit:

Thanks for taking care of this, our builds should all be green now !

@0xazure 0xazure merged commit 39ffb52 into 0xazure:master Nov 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@SeanPrashad SeanPrashad deleted the SeanPrashad:issue-25 branch Nov 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment