Skip to content

Conversation

@GuillaumeGomez
Copy link
Member

r? @rust-lang/docs

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 30, 2017
Copy link
Member

Choose a reason for hiding this comment

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

[00:03:41] tidy error: /checkout/src/libstd/sys/unix/ext/fs.rs:90: trailing whitespace

@GuillaumeGomez
Copy link
Member Author

Updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding ? at the end of the method call instead of ignoring the result?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could just remove the ';'.

Copy link
Member

Choose a reason for hiding this comment

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

Broken link, needs one more ../.

[01:13:11] std/os/unix/fs/trait.FileExt.html:70: broken link - std/std/fs/struct.File.html
[01:13:11] std/os/unix/fs/trait.FileExt.html:95: broken link - std/std/fs/struct.File.html
[01:13:20] thread 'main' panicked at 'found some broken links', /checkout/src/tools/linkchecker/main.rs:49:8
[01:13:20] note: Run with `RUST_BACKTRACE=1` for a backtrace.

Copy link
Contributor

Choose a reason for hiding this comment

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

This will only read up to 8 bytes so I think it's important to do something with the value read_at returns in this example. The same goes for write_at.

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 point.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe something like this?

// Read up to 8 bytes from the offset 10.
let num_bytes_read = file.read_at(buf, 10)?;
println!("read {} bytes: {:?}", num_bytes_read, buf);

@carols10cents
Copy link
Member

r? @frewsxcv

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this let mut buf = [0u8; 8]; and pass &mut buf to read_at? That way it'll match what we have in std::io::Read.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hum sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you use read.v instead of method.read? I know they go to the same spot but i'm more used to reading the method. style link anchors. (Same for write farther down.)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not targetting the same thing actually. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I would consider "the first child element of a container versus the container itself" to be close enough.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Hum, good point.

Copy link
Contributor

@frewsxcv frewsxcv left a comment

Choose a reason for hiding this comment

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

r=me with @QuietMisdreavus's and @ollie27's comments addressed

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe something like this?

// Read up to 8 bytes from the offset 10.
let num_bytes_read = file.read_at(buf, 10)?;
println!("read {} bytes: {:?}", num_bytes_read, buf);

Copy link
Contributor

Choose a reason for hiding this comment

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

this is not guaranteed to read 8 bytes, so phrasing it as 'up to 8 bytes' is a bit more accurate

@bors
Copy link
Collaborator

bors commented Nov 8, 2017

☔ The latest upstream changes (presumably #45862) made this pull request unmergeable. Please resolve the merge conflicts.

@frewsxcv
Copy link
Contributor

frewsxcv commented Nov 9, 2017

r=me with merge conflicts addressed

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2017
@GuillaumeGomez
Copy link
Member Author

@bors: r=frewsxcv rollup

@bors
Copy link
Collaborator

bors commented Nov 11, 2017

📌 Commit c09adc4 has been approved by frewsxcv

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 11, 2017
…wsxcv

Add missing links and examples for FileExt

r? @rust-lang/docs
bors added a commit that referenced this pull request Nov 11, 2017
Rollup of 4 pull requests

- Successful merges: #45631, #45812, #45877, #45919
- Failed merges:
@bors bors merged commit c09adc4 into rust-lang:master Nov 11, 2017
@GuillaumeGomez GuillaumeGomez deleted the file-ext-docs branch November 11, 2017 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants