-
Notifications
You must be signed in to change notification settings - Fork 85
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
Add Debug impls and fix clippy warnings #71
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work here @Victor-N-Suadicani!
@@ -1,6 +1,7 @@ | |||
#![forbid(unsafe_code)] | |||
#![deny(unused)] | |||
#![deny(dead_code)] | |||
#![warn(missing_debug_implementations)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit undecided. I don't think we should be adding Debug
implementations to types by default as it slows compile time and bloats the compiled binary.
Do you feel strongly about this? What do you think of removing this line but keeping the new derive
statements below?
#![warn(missing_debug_implementations)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only lints for public types I believe. So it's not every type.
But I don't feel super strongly about it. Though I think it is a good idea.
Your call :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only lints for public types I believe. So it's not every type.
Good point. Thanks. Let's keep it as is then.
9b0c967
to
0eb6318
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Only missing a changelog entry for the added Debug
implementations.
@@ -1,6 +1,7 @@ | |||
#![forbid(unsafe_code)] | |||
#![deny(unused)] | |||
#![deny(dead_code)] | |||
#![warn(missing_debug_implementations)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only lints for public types I believe. So it's not every type.
Good point. Thanks. Let's keep it as is then.
d943be3
to
da0f1b5
Compare
Signed-off-by: Victor Nordam Suadicani <v.n.suadicani@gmail.com>
Signed-off-by: Victor Nordam Suadicani <v.n.suadicani@gmail.com>
Signed-off-by: Victor Nordam Suadicani <v.n.suadicani@gmail.com>
da0f1b5
to
12b6fff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Victor-N-Suadicani. I will cut a new release soon.
Signed-off-by: Victor Nordam Suadicani <v.n.suadicani@gmail.com> Signed-off-by: ackintosh <sora.akatsuki@gmail.com>
Lots of public types are missing
Debug
impls. I've added most of the easy Debug impls and also added#![warn(missing_debug_implementations)]
to the top level.There are also quite a few Clippy warnings that have been fixed.