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::legacy_numeric_consts lints. #3651

Conversation

waywardmonkeys
Copy link
Contributor

Prefer to use the associated constants on the types rather than the legacy items like std::<type>::MAX or functions like max_value. These legacy items are marked to indicate that they will be deprecated in a future version of Rust.

  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@kchibisov
Copy link
Member

I think this lint is not enabled by clippy by default or yet? We'd need it breaking on CI to catch stuff like that in the future and it should be on stable clippy as well.

Prefer to use the associated constants on the types rather than
the legacy items like `std::<type>::MAX` or functions like
`max_value`. These legacy items are marked to indicate that they
will be deprecated in a future version of Rust.
@waywardmonkeys
Copy link
Contributor Author

I think this lint is not enabled by clippy by default or yet? We'd need it breaking on CI to catch stuff like that in the future and it should be on stable clippy as well.

It is coming. It is present in clippy but not enabled by default in stable, but is enabled in nightly and should be making its way to stable ...

I've rebased the changes to fix the merge conflict and force pushed it.

@kchibisov
Copy link
Member

kchibisov commented Apr 26, 2024

I'm just saying that without automatic way to check for that there's no way we ensure that it'll stay the same.

In general, if it'll come to stable, we'll fix our CI once it happens.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

See above, I generally don't mind, but it's a style change which one can easily reentroduce without a lint in place, so should be done when lint is ready.

@waywardmonkeys
Copy link
Contributor Author

So I should close this and not waste any more time?

@kchibisov
Copy link
Member

kchibisov commented Apr 26, 2024

Yes, because the lint is not stable yet and once it's stable we'll fix it along other new lints, because CI will fail.

@kchibisov kchibisov closed this Apr 26, 2024
@waywardmonkeys waywardmonkeys deleted the legacy-numeric-constants branch April 26, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants