You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/05/25 11:07:36 UTC
[GitHub] [arrow-datafusion] alamb opened a new issue, #207: Enable all clippy lints
alamb opened a new issue, #207:
URL: https://github.com/apache/arrow-datafusion/issues/207
*Note*: migrated from original JIRA: https://issues.apache.org/jira/browse/ARROW-10597
The idea is to get the code to the point where all clippy lints can be enabled.
Here is a list of clippy lints that was disabled:
https://github.com/apache/arrow/pull/8666/files#diff-678f16a0c102ed15656bd00d98c47755920c8c4ede296e3bf5c09a0d4f38a42cR19
The goal of this ticket is to enable them (maybe it would be better to break it into multiple PRs/subtasks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb closed issue #207: Enable all clippy lints
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #207: Enable all clippy lints
URL: https://github.com/apache/arrow-datafusion/issues/207
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb closed issue #207: Enable all clippy lints
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #207: Enable all clippy lints
URL: https://github.com/apache/arrow-datafusion/issues/207
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] waynexia commented on issue #207: Enable all clippy lints
Posted by "waynexia (via GitHub)" <gi...@apache.org>.
waynexia commented on issue #207:
URL: https://github.com/apache/arrow-datafusion/issues/207#issuecomment-1556633950
The command I use to run clippy is `cargo clippy --workspace --all-targets -- -D warnings`. Besides those manually "`allow`ed", our codebase now doesn't contain clippy warnings
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] LouisGariepy commented on issue #207: Enable all clippy lints
Posted by "LouisGariepy (via GitHub)" <gi...@apache.org>.
LouisGariepy commented on issue #207:
URL: https://github.com/apache/arrow-datafusion/issues/207#issuecomment-1562165875
> If someone has time to remove any manual `allow`s that would be awesome
@alamb I'm currently working on a PR that removes a large amount of either unneeded or trivially fixable clippy `allow`s.
Some of the lints *should* be fixed, but are either breaking or generally introduce changes that I'm not comfortable making fixes for. These ones I commented to thoroughly clarify the reason for the lint and potential fixes.
I'll submit the PR soon and then we can discuss the specifics.
Cheers!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] LouisGariepy commented on issue #207: Enable all clippy lints
Posted by "LouisGariepy (via GitHub)" <gi...@apache.org>.
LouisGariepy commented on issue #207:
URL: https://github.com/apache/arrow-datafusion/issues/207#issuecomment-1556090418
@alamb is this issue still relevant? I was looking for an issue to chew on as a soft introduction to the code base, but I think this issue has already been solved, at least partially?
I still see a number of granular clippy `allow`s in the current code, but it's not immediately clear if these are intended to be fixed or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on issue #207: Enable all clippy lints
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #207:
URL: https://github.com/apache/arrow-datafusion/issues/207#issuecomment-1557233640
Thank you @LouisGariepy and @waynexia -- I agree this is no longer relevant
I did a double check of clippy lints that were manually `allow`ed : https://github.com/search?q=repo%3Aapache%2Farrow-datafusion+clippy+allow+language%3ARust&type=code&l=Rust
And while some of them (like `redundant_clone` could probably be cleaned up) I didn't see anything drastic required.
If someone has time to remove any manual `allow`s that would be awesome
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org