Skip to content

Option docs conflates panics with safety #36581

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
bluss opened this issue Sep 19, 2016 · 3 comments
Closed

Option docs conflates panics with safety #36581

bluss opened this issue Sep 19, 2016 · 3 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@bluss
Copy link
Member

bluss commented Sep 19, 2016

In Rust, safety is memory safety, and panic is safe. Option docs should not use "safety" for this notice: https://doc.rust-lang.org/nightly/std/option/enum.Option.html#safety-note
@bluss bluss added the A-docs label Sep 19, 2016
@sfackler
Copy link
Member

Should probably just retitle that section to "Warning" or "Note" or something like that.

@steveklabnik
Copy link
Member

steveklabnik commented Sep 19, 2016 •
edited
Loading

Yes, this seems sub-par. I would remove the heading and move it into the regular description, personally. If anyone would like to fix this, I would be happy to mentor.

@steveklabnik steveklabnik added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-help-wanted Call for participation: Help is requested to fix this issue. labels Sep 19, 2016
@jpadkins
Copy link
Contributor

jpadkins commented Sep 19, 2016 •
edited
Loading

I'd be interested in fixing this @steveklabnik

To clarify:

  • remove the Safety note header
  • move the text In general, because... to be directly under the unwrap definition,
    so that it's right after the text Moves the value...

@jpadkins jpadkins mentioned this issue Sep 19, 2016
Merged
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 21, 2016
Rollup merge of rust-lang#36589 - jpadkins:option_docs_safety_wording…
5cd252c 
…_fix, r=bluss

fixed the safety header/wording in option.rs

Fixes rust-lang#36581

screenshot of the rendered documentation: http://imgur.com/14kLVrA

r? @steveklabnik
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 21, 2016
Rollup merge of rust-lang#36589 - jpadkins:option_docs_safety_wording…
ae315da 
…_fix, r=bluss

fixed the safety header/wording in option.rs

Fixes rust-lang#36581

screenshot of the rendered documentation: http://imgur.com/14kLVrA

r? @steveklabnik
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 22, 2016
Rollup merge of rust-lang#36589 - jpadkins:option_docs_safety_wording…
5d4c0ed 
…_fix, r=bluss

fixed the safety header/wording in option.rs

Fixes rust-lang#36581

screenshot of the rendered documentation: http://imgur.com/14kLVrA

r? @steveklabnik
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@steveklabnik @sfackler @bluss @jpadkins