-
Notifications
You must be signed in to change notification settings - Fork 325
Remove main function from doctests #752
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
Conversation
In order to use the ? operator, we need to make it clear the function returns a result instead of unit: https://doc.rust-lang.org/rustdoc/documentation-tests.html#using--in-doc-tests
src/impl_constructors.rs
Outdated
@@ -132,18 +132,13 @@ where | |||
/// use approx::assert_abs_diff_eq; | |||
/// use ndarray::{Array, arr1}; | |||
/// | |||
/// # fn example() -> Option<()> { |
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.
Does this cause a clippy
failure? Given that the return type is Option<()>
I wouldn't an expect this to be deemed a needless_doctest_main
.
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.
Lemme double-check why I've done so. It might be for the sake of simplicity for the example, but I agree it wouldn't be the exact scope of this pull request.
Given I couldn't find an explanation in the commit message (I usually leave explanatory messages), going to re-run it.
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.
Yep, running fine with the example function. So it was, indeed, a matter of simplicity
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.
Looks good ⭐ Thank you! 🙏
This pull requests addresses the needless_doctest_main linting issues.
Most of the remaining ones are with regards missing
Safety
section in the docs, using clippy words, missing_safety_doc, which was added by rust-lang/rust-clippy#4608. Those aren't as straightforward for fixing as they require a per-case analysis in order to verify the safety pre-requisites.There is also one occurrency of a redundant clone