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

Improve CI #328

Closed
wants to merge 1 commit into from
Closed

Improve CI #328

wants to merge 1 commit into from

Conversation

lovasoa
Copy link
Contributor

@lovasoa lovasoa commented Jul 2, 2019

  • Allow warnings on rust nightly (this allows keeping compatibility with both rust 1.20 and nightly)
  • Create and independent script for CI instead of writing code inside .travis.yml
  • Fix warnings in the tests

Fixes #327

 - Allow warnings on rust nightly (this allows keeping compatibility with both rust 1.20 and nightly)
 - Create and independent script for CI instead of writing code inside .travis.yml
 - Fix warnings in the tests
@seanmonstar
Copy link
Member

I think this means that when that lint makes it to beta (in a couple days?), we'll see it triggered there as well.

How about instead of removing the deny(warnings) entirely, just adding an allow where a warning we can't fix it triggered. Like:

#[allow(warnings)]
type AnyMap = HashMap<TypeId, Box<Any + Send + Sync>, BuildHasherDefault<IdHasher>>;

@lovasoa
Copy link
Contributor Author

lovasoa commented Jul 3, 2019

How about instead of removing the deny(warnings) entirely, just adding an allow where a warning we can't fix it triggered.

#[allow(warnings)] would allow any kind of warning on the portion of code it is applied to, not just compatibility warnings. Is that really what we want ?

I think this means that when that lint makes it to beta (in a couple days?), we'll see it triggered there as well.

Maybe warnings should be forbidden only on stable, then ?

@lovasoa
Copy link
Contributor Author

lovasoa commented Jul 3, 2019

on a side note: another change that this PR introduces is that warnings are disallowed even in tests.

@seanmonstar
Copy link
Member

would allow any kind of warning on the portion of code it is applied to, not just compatibility warnings. Is that really what we want ?

Yep, that's fine. It's a smaller change, and there's no concern of other warnings being triggered.

@lovasoa
Copy link
Contributor Author

lovasoa commented Jul 5, 2019

Feel free to close or edit this PR if you think it is not relevant, then.

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.

Compilation is broken on nightly
2 participants