Skip to content

Improve docs for mem::forget() #27521

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
Aug 5, 2015

Conversation

steveklabnik
Copy link
Member

We were burying the reason to use this function below a bunch of caveats about
its usage. That's backwards. Why a function should be used belongs at the top of
the docs, not the bottom.

Also, add some extra links to related functions mentioned in the body.

/cc @abhijeetbhagat who pointed this out on IRC

@rust-highfive
Copy link
Contributor

r? @brson

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

@Gankra
Copy link
Contributor

Gankra commented Aug 4, 2015

The PR says mem::swap but this seems to be a mem::forget change?

@steveklabnik
Copy link
Member Author

lol i am the worst

@steveklabnik steveklabnik changed the title Improve docs for mem::swap() Improve docs for mem::forget() Aug 4, 2015
@@ -84,7 +86,7 @@ pub use intrinsics::transmute;
/// mem::forget(file);
/// ```
///
/// The swap function uses forget to good effect.
/// The `mem::swap()` function uses forget to good effect:
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to be weird to include parens here, especially since it actually takes args.

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've tended to always use parens when talking about a function(). I guess that's not official style...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I generally don't...? In this case it's clear it's a function anyway.

Copy link
Member

Choose a reason for hiding this comment

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

you wrapped mem::swap inside the ``, but not mem::forget?

@Gankra
Copy link
Contributor

Gankra commented Aug 4, 2015

r=me with nit

@steveklabnik steveklabnik force-pushed the doc_std_mem_forget branch 2 times, most recently from 2fd7110 to 5af1b3f Compare August 4, 2015 20:45
@steveklabnik
Copy link
Member Author

@bors: r=gankro rollup

@bors
Copy link
Collaborator

bors commented Aug 4, 2015

📌 Commit 5af1b3f has been approved by gankro

We were burying the reason to use this function below a bunch of caveats about
its usage. That's backwards. Why a function should be used belongs at the top of
the docs, not the bottom.

Also, add some extra links to related functions mentioned in the body.
Manishearth added a commit to Manishearth/rust that referenced this pull request Aug 5, 2015
…gankro

We were burying the reason to use this function below a bunch of caveats about
its usage. That's backwards. Why a function should be used belongs at the top of
the docs, not the bottom.

Also, add some extra links to related functions mentioned in the body.

/cc @abhijeetbhagat who  pointed this out on IRC
bors added a commit that referenced this pull request Aug 5, 2015
@bors bors merged commit 5af1b3f into rust-lang:master Aug 5, 2015
@steveklabnik steveklabnik deleted the doc_std_mem_forget branch June 19, 2016 20:33
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.

6 participants