8000 Fix CI failures (mostly linting with clippy) by aganders3 · Pull Request #1171 · rust-ndarray/ndarray · GitHub
[go: up one dir, main page]
Skip to content

Fix CI failures (mostly linting with clippy)#1171

Merged
jturner314 merged 7 commits intorust-ndarray:masterfrom
aganders3:fix-ci
Jul 30, 2022
Merged

Fix CI failures (mostly linting with clippy)#1171
jturner314 merged 7 commits intorust-ndarray:masterfrom
aganders3:fix-ci

Conversation

@aganders3
Copy link
Contributor
@aganders3 aganders3 commented Jun 16, 2022

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
aganders3 commented Jun 16, 2022

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 jturner314 merged commit c66d408 into rust-ndarray:master Jul 30, 2022
@jturner314
Copy link
Member
jturner314 commented Jul 30, 2022 •
edited
Loading

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 jturner314 mentioned this pull request Jul 30, 2022
Merged
jturner314 added a commit that referenced this pull request Jul 30, 2022
@jturner314
Merge pull request #1190 from jturner314/cherry-pick-1171
dedb15f 
Cherry pick non-breaking portions of #1171 to 0.15.x
@aganders3
Copy link
Contributor Author
aganders3 commented Jul 31, 2022

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

Reviewers

No reviews

Assignees

No one assigned

Labels

breaking-change

4658 Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aganders3 @jturner314
0