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 clippy lints #78

Merged
merged 5 commits into from
Sep 20, 2024
Merged

fix clippy lints #78

merged 5 commits into from
Sep 20, 2024

Conversation

skewballfox
Copy link
Contributor

nothing major, just ran cargo clippy fix, then applied non-automatic fixes manually

Copy link
Owner

@jfrimmel jfrimmel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just one question:

src/main.rs Outdated Show resolved Hide resolved
@skewballfox
Copy link
Contributor Author

should I update the MSRV?

@skewballfox
Copy link
Contributor Author

@jfrimmel using cargo-msrv it looks like the minimum supportable rust version that keeps the fixes is 1.58.1. I can update that in .github/workflows/msrv.yaml if you like. otherwise I can roll back the lints that are failing.

@jfrimmel
Copy link
Owner

I think it would be fine to raise the MSRV, as it is promised nowhere as of now. So: go for it 😉

@jfrimmel
Copy link
Owner

jfrimmel commented Sep 20, 2024

Ah, it looks like I've screwed up the CI configuration. I've pulled in the latest CI changes. This allows me to run the missing workflows.
Sorry for the inconvenience!

@skewballfox
Copy link
Contributor Author

hey I updated the MSRV both in .clippy.toml and .github/msrv.yaml, and reran clippy fix. should be good to go

@jfrimmel jfrimmel merged commit 3084430 into jfrimmel:master Sep 20, 2024
10 checks passed
@jfrimmel
Copy link
Owner

Thank you very much!

jfrimmel added a commit that referenced this pull request Sep 23, 2024
This is possible thanks to the raised MSRV in #78. Many dependencies are
not updated, though, as the MSRV is still rather old and, unfortunately,
not all dependencies declare a MSRV, thus an automatic `cargo update` is
not possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants