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

Document std::fs::File close behavior ignoring errors #59470

Merged
merged 4 commits into from
Apr 4, 2019
Merged

Document std::fs::File close behavior ignoring errors #59470

merged 4 commits into from
Apr 4, 2019

Conversation

czipperz
Copy link
Contributor

Resolves #52685

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2019
@czipperz
Copy link
Contributor Author

As an aside, is there a close method that allows the user to get a std::io::Result for closing a card? That seems like it would be a nice feature.

@Centril
Copy link
Contributor

Centril commented Mar 30, 2019

Ping from triage, @dtolnay :)

@czipperz
Copy link
Contributor Author

czipperz commented Mar 30, 2019 •
edited
Loading

@Centril What do you think about adding a close method to File?

fn close(self) -> io::Result<()>

@Centril
Copy link
Contributor

Centril commented Mar 30, 2019

@czipperz I have no opinion. :)

@pnkfelix pnkfelix added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 1, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Apr 1, 2019 •
edited
Loading

@Centril What do you think about adding a close method to File?

fn close(self) -> io::Result<()>

@dtolnay has previously stated that such a change requires going through the RFC process. See discussion at #32255

Update: and I see the PR author has actually opened an issue dedicated to this question, at #59567

dtolnay
dtolnay requested changes Apr 2, 2019
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

src/libstd/fs.rs Outdated Show resolved Hide resolved
src/libstd/fs.rs Outdated Show resolved Hide resolved
src/libstd/fs.rs Outdated Show resolved Hide resolved
src/libstd/fs.rs Outdated Show resolved Hide resolved
@czipperz
Copy link
Contributor Author

czipperz commented Apr 2, 2019

@dtolnay Redid the documentation based on comments I've read in other threads.

dtolnay
dtolnay approved these changes Apr 2, 2019
View reviewed changes
src/libstd/fs.rs Outdated Show resolved Hide resolved
dtolnay
dtolnay approved these changes Apr 4, 2019
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me.

@dtolnay
Copy link
Member

dtolnay commented Apr 4, 2019

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 4, 2019

📌 Commit a969d40 has been approved by dtolnay

@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 Apr 4, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 4, 2019
@Centril
Rollup merge of rust-lang#59470 - czipperz:document-fs-file-close, r=…
a3a0b7f 
…dtolnay

Document std::fs::File close behavior ignoring errors

Resolves rust-lang#52685
@Centril Centril mentioned this pull request Apr 4, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Apr 4, 2019
@Centril
Rollup merge of rust-lang#59470 - czipperz:document-fs-file-close, r=…
2e83edc 
…dtolnay

Document std::fs::File close behavior ignoring errors

Resolves rust-lang#52685
@Centril Centril mentioned this pull request Apr 4, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Apr 4, 2019
@Centril
Rollup merge of rust-lang#59470 - czipperz:document-fs-file-close, r=…
bfb7f34 
…dtolnay

Document std::fs::File close behavior ignoring errors

Resolves rust-lang#52685
@Centril Centril mentioned this pull request Apr 4, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Apr 4, 2019
@Centril
Rollup merge of rust-lang#59470 - czipperz:document-fs-file-close, r=…
02de76e 
…dtolnay

Document std::fs::File close behavior ignoring errors

Resolves rust-lang#52685
@Centril Centril mentioned this pull request Apr 4, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Apr 4, 2019
@Centril
Rollup merge of rust-lang#59470 - czipperz:document-fs-file-close, r=…
886456d 
…dtolnay

Document std::fs::File close behavior ignoring errors

Resolves rust-lang#52685
@Centril Centril mentioned this pull request Apr 4, 2019
Merged
bors added a commit that referenced this pull request Apr 4, 2019
@bors
Auto merge of #59695 - Centril:rollup-88qffc2, r=Centril
52980d0 
Rollup of 8 pull requests

Successful merges:

 - #59470 (Document std::fs::File close behavior ignoring errors)
 - #59555 (update miri)
 - #59556 (update stdsimd)
 - #59596 (Forward formatter settings to bounds of `Range<T>` in `fmt::Debug` impl)
 - #59639 (Never return uninhabited values at all)
 - #59671 (Make some of lexer's API private)
 - #59685 (Add description for -Os and -Oz in rustc.1)
 - #59686 (Temporarily disable stack probing for gnux32.)

Failed merges:

r? @ghost
@bors bors merged commit a969d40 into rust-lang:master Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

Labels
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@czipperz @rust-highfive @Centril @pnkfelix @dtolnay @bors