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

Use #![warn(single_use_lifetimes)] #1680

Merged
merged 6 commits into from
Jun 25, 2019

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Jun 19, 2019

I tried to do this at the time of #1531, but I couldn't do it because of rust-lang/rust#53738 (It needed too much #[allow(single_use_lifetimes)]...).

@taiki-e
Copy link
Member Author

taiki-e commented Jun 19, 2019

Hmm... this causes many warnings in older versions: https://travis-ci.com/rust-lang-nursery/futures-rs/jobs/209181035

@@ -9,6 +9,8 @@
#![cfg_attr(not(feature = "std"), no_std)]

#![warn(missing_docs, missing_debug_implementations, rust_2018_idioms, unreachable_pub)]
// It cannot be included in the published code because this lints have false positives in the minimum required version.
#![cfg_attr(test, warn(single_use_lifetimes))]
Copy link
Member Author

Choose a reason for hiding this comment

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

This enables single_use_lifetimes lint when running cargo test, cargo check --tests etc. (FWIW, I often run cargo check --all-features --tests --all locally before opening PR).

@taiki-e
Copy link
Member Author

taiki-e commented Jun 19, 2019

There are still warnings at cargo +nightly test (minimum required version) and cargo test (with minimal versions): https://travis-ci.com/rust-lang-nursery/futures-rs/builds/116074146

@taiki-e
Copy link
Member Author

taiki-e commented Jun 19, 2019

cargo +nightly test (minimum required version) and cargo test (with minimal versions)

Given the reason for checking them, cargo build is enough, IMO.

@@ -22,4 +22,4 @@ std = []
proc-macro2 = "0.4"
proc-macro-hack = "0.5.3"
quote = "0.6"
syn = { version = "0.15.22", features = ["full"] }
syn = { version = "0.15.25", features = ["full"] }
Copy link
Member

Choose a reason for hiding this comment

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

Why was this change required?

Copy link
Member Author

@taiki-e taiki-e Jun 19, 2019

Choose a reason for hiding this comment

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

A warning occurred when building with minimal versions: https://travis-ci.com/rust-lang-nursery/futures-rs/jobs/209211855. (that warning was fixed in dtolnay/syn#563, and that change is included in 0.15.25 and later versions)

(Context: After the first commit, I realized that CI did not catch(and reject) the warnings, so I added eb4cbfa and changed the warnings to be denied for all builds.)

@cramertj cramertj merged commit 8e50507 into rust-lang:master Jun 25, 2019
@taiki-e taiki-e deleted the single_use_lifetimes branch June 25, 2019 16:37
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jun 26, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jun 26, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jun 29, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jul 2, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jul 2, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jul 7, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jul 14, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jul 19, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jul 20, 2019
taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Jul 27, 2019
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