Skip to content

doc: miscellaneous OpenOptions:append improvements #31093

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

Merged
merged 1 commit into from
Jan 24, 2016
Merged

doc: miscellaneous OpenOptions:append improvements #31093

merged 1 commit into from
Jan 24, 2016

Conversation

tshepang
Copy link
Member

No description provided.

@rust-highfive
Copy link
Contributor

r? @nikomatsakis

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

@@ -436,19 +436,19 @@ impl OpenOptions {
/// Note that setting `.write(true).append(true)` has the same effect as
/// setting only `.append(true)`.
///
/// For most filesystems the operating system guarantees all writes are
/// For most filesystems, the operating system guarantees all writes to be
Copy link
Member

Choose a reason for hiding this comment

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

This actually sounds worse to my ear, is there any precedence anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

An alternative is "the operating system guarantees that all writes are atomic".

Copy link
Member

Choose a reason for hiding this comment

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

👍

@steveklabnik
Copy link
Member

I like it all except that one thing.

/// can be done by concatenating strings before passing them to `write()`,
/// or using a buffered writer (with a more than adequately sized buffer)
/// or using a buffered writer (with a more than adequately-sized buffer),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe a - is needed between an adverb and the adjective it modifies ("adequately-sized" doesn't look "correct" to me, in other words). However, this kind of "multi-word adjective" -- "more than adequately sized" -- always feels a little awkward and informal to me in any case. Perhaps "with a buffer of sufficient size" or "with a sufficiently large buffer". (Also, I have no idea what a "more than adequately sized buffer" even is supposed to mean -- surely "adequate" size would be enough, since that is sort of the definition of adequate?)

Copy link
Member Author

Choose a reason for hiding this comment

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

good points

@tshepang
Copy link
Member Author

review comments addresed

@steveklabnik
Copy link
Member

@bors: r+ rollup

thank you!

@bors
Copy link
Collaborator

bors commented Jan 24, 2016

📌 Commit 012d68a has been approved by steveklabnik

@bors
Copy link
Collaborator

bors commented Jan 24, 2016

⌛ Testing commit 012d68a with merge 4043c02...

@bors bors merged commit 012d68a into rust-lang:master Jan 24, 2016
@tshepang tshepang deleted the misc-doc-improvements branch January 25, 2016 06:14
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.

5 participants