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

Fix CI failures (mostly linting with clippy) #1171

Merged
merged 7 commits into from Jul 30, 2022

Conversation

aganders3
Copy link
Contributor

Hello - I noticed CI has been consistently failing for some time, so this PR is meant to bring it back to green. I think it was mostly (all?) clippy lints that were blocking it. The way the GitHub Actions are set up now this could break again (mostly for more clippy lints) with new versions of Rust. I think it's still good to test like this against new/current versions, and I'm happy to keep an eye out for similar issues in the future.

The most frequent was a lint related to #[doc(hidden)] which (according to clippy) does not work within Trait impl blocks. I basically just deleted all of them and also combined the private_decl and private_impl macros - it seems to me the private_decl can serve the full purpose of making the Trait private and providing a default implementation. Please let me know if I got this wrong or you'd like to take a different track.

@aganders3
Copy link
Contributor Author

Arg, just saw #1165 that already does a good chunk of this!

@aganders3 aganders3 changed the title Fix CI Tests (mostly linting with clippy) Fix CI failures (mostly linting with clippy) Jun 17, 2022
@jturner314 jturner314 force-pushed the fix-ci branch 3 times, most recently from 224a75c to 160644b Compare July 30, 2022 19:24
@jturner314 jturner314 changed the base branch from master to 0.15.x July 30, 2022 19:42
@jturner314 jturner314 force-pushed the fix-ci branch 2 times, most recently from 3cd5cb1 to 5feca5f Compare July 30, 2022 19:46
@jturner314 jturner314 changed the base branch from 0.15.x to master July 30, 2022 19:48
@jturner314 jturner314 force-pushed the fix-ci branch 2 times, most recently from 1200141 to 1a30604 Compare July 30, 2022 19:49
@jturner314
Copy link
Member

jturner314 commented Jul 30, 2022

Thanks for the fixes! I merged #1165 into master, rebased this PR off the latest master, moved the MSRV addition into a separate commit, and added a few more fixes. I dropped the changes which combined the private_decl! and private_impl! macros, since they made it possible for users to implement the corresponding traits. (The purpose of those macros is to prevent some of ndarray's traits from being implementable by downstream crates.)

I haven't investigated the remaining cross_test failures; based on the error message, it may be an issue with GitHub CI which will resolve itself. If anyone has any ideas how to fix it, please create a PR.

Note on the breaking-change label: This PR is a breaking change for the 0.15.x series, since it requires a Rust compiler newer than 1.49.

jturner314 added a commit that referenced this pull request Jul 30, 2022
Cherry pick non-breaking portions of #1171 to 0.15.x
@aganders3
Copy link
Contributor Author

Oh wow - thanks for getting this into shape and accepting it! I really appreciate your work on this crate.

I dropped the changes which combined the private_decl! and private_impl! macros, since they made it possible for users to implement the corresponding traits. (The purpose of those macros is to prevent some of ndarray's traits from being implementable by downstream crates.)

Ah, okay. I'll have to look over exactly how this works but that makes sense.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants