Skip to content
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

Add documentation for the order of Option and Result #87654

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

jesyspa
Copy link
Contributor

@jesyspa jesyspa commented Jul 31, 2021

This resolves issue #87238.

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2021
@scottmcm
Copy link
Member

It's not obvious to me that the manual implementation is worth it, here. There's also something to be said for people seeing that it's just the derived one.

Adding documentation (and doctests) about it is definitely good, though. Maybe, as a compromise, add another section to the module level docs about how Ord/Eq/Hash/etc work with it? There are a bunch that were added recently, and this feels like it would fit that pattern well.

@jesyspa jesyspa marked this pull request as draft July 31, 2021 21:21
@jesyspa
Copy link
Contributor Author

jesyspa commented Jul 31, 2021

Thanks, that makes sense, let me write it up there :)

@jesyspa jesyspa force-pushed the issue-87238-option-result-doc branch from 05c53cc to 60ddd6b Compare August 1, 2021 11:21
@jesyspa jesyspa force-pushed the issue-87238-option-result-doc branch from 7fe3e07 to 40eaab1 Compare August 1, 2021 11:59
@jesyspa jesyspa changed the title Explicitly implement and document comparison for Option and Result Add document for the order of Option and Result Aug 1, 2021
@jesyspa jesyspa marked this pull request as ready for review August 1, 2021 12:00
@jesyspa jesyspa changed the title Add document for the order of Option and Result Add documentation for the order of Option and Result Aug 1, 2021
@scottmcm
Copy link
Member

scottmcm commented Aug 2, 2021

r? @scottmcm
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 2, 2021

📌 Commit 40eaab1 has been approved by scottmcm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 2, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#86176 (Implement a `explicit_generic_args_with_impl_trait` feature gate)
 - rust-lang#87654 (Add documentation for the order of Option and Result)
 - rust-lang#87659 (Fix invalid suggestions for non-ASCII characters in byte constants)
 - rust-lang#87673 (Tweak opaque type mismatch error)
 - rust-lang#87687 (Inline some macros)
 - rust-lang#87690 (Add missing "allocated object" doc link to `<*mut T>::add`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b1166e1 into rust-lang:master Aug 2, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 2, 2021
@jesyspa jesyspa deleted the issue-87238-option-result-doc branch August 20, 2021 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants