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.

Sign up for GitHub

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

Jump to bottom

Improve the "must use" lint for Future #60808

Merged
merged 1 commit into from
May 14, 2019
Merged

Improve the "must use" lint for Future #60808

merged 1 commit into from
May 14, 2019

Conversation

Schultzer
Copy link
Contributor

Fixes #60797

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @withoutboats (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2019
@Centril
Copy link
Contributor

Centril commented May 13, 2019

r? @Centril

@Schultzer Schultzer force-pushed the improve-must-use-linit-for-future branch from 3ef221d to a3ad793 Compare May 13, 2019 22:31
@cramertj
Copy link
Member

It's probably worth calling out here that .await is not the only way to poll a future-- there are many others, like polling directly, passing it to an executor, or using it inside the join! or select! macros. I think it's helpful to point users towards await!, but it might be worth thinking about how to clarify that await isn't the only thing you can do.

@Centril
Copy link
Contributor

Centril commented May 13, 2019

@cramertj Open to suggestions on how to point at a broader set, but remember that the message has to be short and sweet.

@Schultzer @Centril
Improve the "must use" lint for Future
58c6a94 
Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>
@Schultzer Schultzer force-pushed the improve-must-use-linit-for-future branch from a3ad793 to 58c6a94 Compare May 13, 2019 23:01
@Centril
Copy link
Contributor

Centril commented May 13, 2019

@bors r+ rollup

There may be some tests that need to be --blessed but I'll sign off optimistically for now.

@cramertj If you think we need to make more changes, feel free to send in another PR or r- if you have a better wording on your mind now.

@bors
Copy link
Contributor

bors commented May 13, 2019

📌 Commit 58c6a94 has been approved by Centril

@bors
Copy link
Contributor

bors commented May 13, 2019

🌲 The tree is currently closed for pull requests below priority 500, this pull request will be tested once the tree is reopened

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 13, 2019
@cramertj
Copy link
Member

nah the updated message seems fine to me, thanks :)

taiki-e added a commit to taiki-e/futures-rs that referenced this pull request May 14, 2019
@taiki-e
Update #[must_use] message of impl Future types
75ef1db 
Refs: rust-lang/rust#60808
Centril added a commit to Centril/rust that referenced this pull request May 14, 2019
@Centril
Rollup merge of rust-lang#60808 - Schultzer:improve-must-use-linit-fo…
7dbf37b 
…r-future, r=Centril

Improve the "must use" lint for `Future`

Fixes rust-lang#60797
@Centril Centril mentioned this pull request May 14, 2019
Merged
bors added a commit that referenced this pull request May 14, 2019
@bors
Auto merge of #60834 - Centril:rollup-fikyi9i, r=Centril
372be4f 
Rollup of 9 pull requests

Successful merges:

 - #60130 (Add implementations of last in terms of next_back on a bunch of DoubleEndedIterators)
 - #60443 (as_ptr returns a read-only pointer)
 - #60444 (forego caching for all participants in cycles, apart from root node)
 - #60719 (Allow subdirectories to be tested by x.py test)
 - #60780 (fix Miri)
 - #60788 (default to $ARCH-apple-macosx10.7.0 LLVM triple for darwin targets)
 - #60799 (Allow late-bound regions in existential types)
 - #60808 (Improve the "must use" lint for `Future`)
 - #60819 (submodules: update clippy from 3710ec5 to ad3269c)

Failed merges:

r? @ghost
cramertj pushed a commit to rust-lang/futures-rs that referenced this pull request May 14, 2019
@taiki-e @cramertj
Update #[must_use] message of impl Future types
ded685d 
Refs: rust-lang/rust#60808
@bors bors merged commit 58c6a94 into rust-lang:master May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

@varkor varkor varkor left review comments

@cramertj cramertj cramertj left review comments

Assignees

@Centril Centril

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

improve the "must use" lint for Future to suggest await
7 participants
@Schultzer @rust-highfive @Centril @cramertj @bors @varkor @withoutboats