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

Add warning to BufWriter documentation #43136

Merged
merged 1 commit into from
Jul 12, 2017
Merged

Conversation

jgallag88
Copy link
Contributor

When using BufWriter, it is very easy to unintentionally ignore errors, because errors which occur when flushing buffered data when the BufWriter is dropped are ignored. This has been noted in a couple places: #32677, #37045.

There has been some discussion about how to fix this problem in #32677, but no solution seems likely to land in the near future. For now, anyone who wishes to have robust error handling must remember to manually call flush() on a BufWriter before it is dropped. Until a permanent fix is in place, it seems worthwhile to add a warning to that effect to the documentation.

@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 @BurntSushi (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.

@frewsxcv frewsxcv added A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2017
@BurntSushi
Copy link
Member

Adding this warning seems sensible enough to me. I think the only question is whether this now makes the behavior part of the public API, and if so, whether we're comfortable with that or not. I kind of feel like it's already the de facto API, so these docs are just recognizing that.

Anyone else from @rust-lang/libs want to weigh in?

@alexcrichton
Copy link
Member

Seems like a fine change to me!

@steveklabnik
Copy link
Member

@bors: r+ rollup

thank you!

@bors
Copy link
Contributor

bors commented Jul 10, 2017

📌 Commit c6b280e has been approved by steveklabnik

@arielb1 arielb1 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 Jul 11, 2017
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 12, 2017
…bnik

Add warning to BufWriter documentation

When using `BufWriter`, it is very easy to unintentionally ignore errors, because errors which occur when flushing buffered data when the `BufWriter` is dropped are ignored. This has been noted in a couple places:  rust-lang#32677, rust-lang#37045.

There has been some discussion about how to fix this problem in rust-lang#32677, but no solution seems likely to land in the near future. For now, anyone who wishes to have robust error handling must remember to manually call `flush()` on a `BufWriter` before it is dropped.  Until a permanent fix is in place, it seems worthwhile to add a warning to that effect to the documentation.
bors added a commit that referenced this pull request Jul 12, 2017
Rollup of 8 pull requests

- Successful merges: #42670, #42826, #43000, #43011, #43098, #43100, #43136, #43137
- Failed merges:
@bors bors merged commit c6b280e into rust-lang:master Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants