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

make __nutype_{type_name}__ compatible with clippy lint #190

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

vic1707
Copy link
Contributor

@vic1707 vic1707 commented Oct 21, 2024

rust analyzer in VSCode complains about generated nutype module not being proper snake case, this PR fixes that with a naive to_lower_case call as I think nobody really cares about how exactly it is named except clippy x).

{FF067F22-4979-41A5-ABAC-D9099C9E9C7E}

@greyblake
Copy link
Owner

Thanks.
I think we should rather ignore the lint explicitly.

My concern is having 2 different types like

struct Notebook(..)

struct NotEBook(..)

can cause undesired collision in the module names.

@greyblake
Copy link
Owner

@vic1707 Could you please apply #[allow(non_snake_case)] instead?

@vic1707 vic1707 force-pushed the snake_case_nutype_module branch from 854f40d to 2678f38 Compare October 24, 2024 12:49
@vic1707
Copy link
Contributor Author

vic1707 commented Oct 24, 2024

Yeah you're right, I didn't think about it 👍
I chose to #[allow(non_snake_case, reason = "we keep original structure name which is probably CamelCase")].
Since not providing a reason can also trigger a warning x)
{381A8C5E-6318-4F34-890B-291B0178D429}
(yes I pretty much enable every available lint on my projects, sometimes a pain but worth it since I learn a lot and helpful when clippy suggests better versions of my code)

@greyblake
Copy link
Owner

Thank you!

@greyblake greyblake merged commit 7f6e619 into greyblake:master Oct 25, 2024
6 checks passed
@vic1707 vic1707 deleted the snake_case_nutype_module branch October 26, 2024 14:57
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