Skip to content

Improve {String,Vec}::from_raw_parts documentation - #34884

Merged
bors merged 2 commits into
rust-lang:masterfrom
shepmaster:from_raw_parts_doc
Jul 19, 2016
Merged

Improve {String,Vec}::from_raw_parts documentation#34884
bors merged 2 commits into
rust-lang:masterfrom
shepmaster:from_raw_parts_doc

Conversation

@shepmaster

@shepmastershepmaster commented Jul 17, 2016

Copy link
Copy Markdown
Member

No description provided.

@rust-highfive

rust-highfive commented Jul 17, 2016

Copy link
Copy Markdown
Contributor

r? @aturon

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

nagisa
nagisa reviewed Jul 17, 2016
View reviewed changes
Comment threadsrc/libcollections/string.rs Outdated

@nagisanagisaJul 17, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should say “dropped” instead “goes out of scope”. Going out of scope does not necessarily imply the drop (we do not guarantee that, for example) and its the dropping which handles the deallocation.

@shepmastershepmasterJul 17, 2016

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Done.

shepmaster
shepmaster reviewed Jul 17, 2016
View reviewed changes
Comment threadsrc/libcollections/string.rs Outdated

@shepmastershepmasterJul 17, 2016

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Github overzealously hid the unresolved previous conversation

@nagisa

nagisa commented Jul 17, 2016

Copy link
Copy Markdown
Member

Seems good to me.

r=me.

@nagisa

nagisa commented Jul 17, 2016

Copy link
Copy Markdown
Member

Oh, eh, I wonder whether the added paragraph is best under the Safety section. Maybe it would be better to add it just before it (right after the short description)?

@GuillaumeGomez

GuillaumeGomez commented Jul 17, 2016•
edited
Loading

Copy link
Copy Markdown
Member

After discussion on IRC:

@bors: r=nagisa rollup

Thanks @shepmaster!

@bors

bors commented Jul 17, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 661187a has been approved by @nagisa

@bors

bors commented Jul 17, 2016

Copy link
Copy Markdown
Collaborator

💡 This pull request was already approved, no need to approve it again.

@bors

bors commented Jul 17, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 661187a has been approved by @nagisa

@sanxiynsanxiyn mentioned this pull request Jul 18, 2016
Merged
sanxiyn added a commit to sanxiyn/rust that referenced this pull request Jul 18, 2016
@sanxiyn
Rollup merge ofrust-lang#34884- shepmaster:from_raw_parts_doc, r=@n…
1132a4d
…agisa
Improve {String,Vec}::from_raw_parts documentation
bors added a commit that referenced this pull request Jul 19, 2016
@bors
Auto merge of#34898- sanxiyn:rollup, r=sanxiyn
27e766d
Rollup of 5 pull requests
- Successful merges: #34807, #34853, #34875, #34884, #34889
- Failed merges:
@bors
bors merged commit 661187a into rust-lang:masterJul 19, 2016
@shepmaster
shepmaster deleted the from_raw_parts_doc branch July 19, 2016 18:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@aturonaturon

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@shepmaster@rust-highfive@nagisa@GuillaumeGomez@bors@aturon