-
Notifications
You must be signed in to change notification settings - Fork 13.3k
implicit Option
-returning doctests
#61279
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
Conversation
This distinguishes `Option` and `Result`-returning doctests with implicit `main` method, where the former tests must end with `Some(())`.
Looks good to me except the backquote stuff. I don't think a feature gate is needed in here so just r=me once you've updated. Thanks! |
@bors: r+ |
📌 Commit 6bb6c00 has been approved by |
…s, r=GuillaumeGomez implicit `Option`-returning doctests This distinguishes `Option` and `Result`-returning doctests with implicit `main` method, where the former tests must end with `Some(())`. Open question: Does this need a feature gate? r? @GuillaumeGomez
Rollup of 11 pull requests Successful merges: - #60802 (upgrade rustdoc's `pulldown-cmark` to 0.5.2) - #60839 (Fix ICE with struct ctors and const generics.) - #60850 (Stabilize RefCell::try_borrow_unguarded) - #61231 (Fix linkage diagnostic so it doesn't ICE for external crates) - #61244 (Box::into_vec: use Box::into_raw instead of mem::forget) - #61279 (implicit `Option`-returning doctests) - #61280 (Revert "Disable solaris target since toolchain no longer builds") - #61284 (Update all s3 URLs used on CI with subdomains) - #61321 (libsyntax: introduce 'fn is_keyword_ahead(dist, keywords)'.) - #61322 (ci: display more debug information in the init_repo script) - #61333 (Fix ICE with APIT in a function with a const parameter) Failed merges: - #61304 (Speed up Azure CI installing Windows dependencies) r? @ghost
Rollup of 11 pull requests Successful merges: - #60802 (upgrade rustdoc's `pulldown-cmark` to 0.5.2) - #60839 (Fix ICE with struct ctors and const generics.) - #60850 (Stabilize RefCell::try_borrow_unguarded) - #61231 (Fix linkage diagnostic so it doesn't ICE for external crates) - #61244 (Box::into_vec: use Box::into_raw instead of mem::forget) - #61279 (implicit `Option`-returning doctests) - #61280 (Revert "Disable solaris target since toolchain no longer builds") - #61284 (Update all s3 URLs used on CI with subdomains) - #61321 (libsyntax: introduce 'fn is_keyword_ahead(dist, keywords)'.) - #61322 (ci: display more debug information in the init_repo script) - #61333 (Fix ICE with APIT in a function with a const parameter) Failed merges: - #61304 (Speed up Azure CI installing Windows dependencies) r? @ghost
It would have been nice to at least have a FCP. I don't really agree with this. |
@ollie27: For me it's not really an issue. However, do you think we should implement |
Well what does the rest of @rust-lang/rustdoc think about this change? |
@ollie27 Interesting. For the record, I personally think we should add a The goal of my previous Using |
I actually opened #61360 to start a discussion about it. It *seems* like we'd want to have |
Late to the party, but I'm actually opposed to this, since All our messaging up to this point has been that doctests wrap In effect, i pretty much agree with this comment from the other thread. There are other things we can suggest people to do instead of supporting |
```ignore | ||
/// ``` | ||
/// let _ = &[].iter().next()?; | ||
///# Some(()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this line should have added a space between /// #
, since historically having single lines without that leading space has messed up rustdoc's Markdown parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping @llogiq
Removing relnotes label since this was reverted. |
This distinguishes
Option
andResult
-returning doctests with implicitmain
method, where the former tests must end withSome(())
.Open question: Does this need a feature gate?
r? @GuillaumeGomez