Fix clippy issues, enforce in CI #86
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
| } | ||
|
|
||
| /// A no-op implementation of the [`EventPublisher`] trait. | ||
| #[allow(dead_code)] // This is unused when rabbitmq feature is enabled |
There was a problem hiding this comment.
Why not cfg-gate it on not(feature = "events-rabbitmq") then?
There was a problem hiding this comment.
yeah much better, done
There was a problem hiding this comment.
Should be okay for now, but that's a reminder that before we have the codebases diverge even further, can should finally upstream PaginatedKVStore to LDK and add SqliteStore/PostgresStore there, too.
There was a problem hiding this comment.
Yeah good call out, will start on that soon
Anyitechs
left a comment
Anyitechs
left a comment
There was a problem hiding this comment.
Looking good! Thanks for the clean ups.
| } | ||
| } | ||
|
|
||
| #[allow(clippy::too_many_arguments)] |
There was a problem hiding this comment.
I believe this is one of the exceptions being mentioned in the commit message as refactoring the function here is more suitable than suppressing the warning. Could you add a TODO here?
There was a problem hiding this comment.
no this makes more sense to keep as is imo
| # enabled with --all-features. Proto generation is a developer-only tool that | ||
| # requires external dependencies (protoc) and shouldn't be triggered accidentally. | ||
| # This lint configuration tells Cargo that genproto is an expected custom cfg. | ||
| [lints.rust] |
There was a problem hiding this comment.
Not sure I understand this correctly, but running the command to generate updated proto objects usually introduces some format changes to all the existing generated rust code, and will need you to re-format each file manually. Does this also fix that or is there a way we can handle that here?
There was a problem hiding this comment.
you are just supposed to run cargo fmt after you generate, not really relevant here though
There was a problem hiding this comment.
Right. Just wanted to know if there's a way to make it skip re-formatting files that weren't touched, but I agree it's not relevant here
benthecarman
force-pushed
the
clippy
branch
3 times, most recently
from
ab49b50 to
f466ce2
Compare
We weren't checking for any clippy issues. This fixes most of them and adds a few exceptions for ones not worth fixing or ones that will be fixed with future changes.
4 participants
We weren't checking for any clippy issues. This fixes most of them and adds a few exceptions for ones not worth fixing or ones that will be fixed with future changes.