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

ARROW-9192: [CI][Rust] Add support for running clippy #7501

Closed
wants to merge 2 commits into from

Conversation

houqp
Copy link
Member

@houqp houqp commented Jun 20, 2020

The goal of this PR is not to fix all the clippy errors/warnings in arrow crate, but to make sure no new linting errors are introduced in the future so we can slowly fix the remaining errors.

@houqp houqp changed the title run clippy to lint arrow crate in CI ARROW-9192: [Rust] run clippy to lint arrow crate in CI Jun 20, 2020
@github-actions
Copy link

Copy link
Contributor

@paddyhoran paddyhoran left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @houqp.

@kszucs can you take a look at this also please when you get a chance.

@wesm
Copy link
Member

wesm commented Jun 24, 2020

Hm I think this lint step should be merged into the main Lint workflow. @kszucs can you help?

@houqp
Copy link
Member Author

houqp commented Jun 24, 2020

@kszucs let me know if there is anything i can help to move it to the main lint workflow.

@houqp
Copy link
Member Author

houqp commented Jul 2, 2020

@kszucs gentle ping, let me know what's the best way for me to help.

@kszucs
Copy link
Member

kszucs commented Jul 2, 2020

Hm I think this lint step should be merged into the main Lint workflow. @kszucs can you help?

It would be nice to have it integrated to archery, but since it's only triggered for the rust changes I think it's fine to defer it to a follow-up.

Created ARROW-9295: [Archery] Support rust clippy in the lint command.

@kszucs
Copy link
Member

kszucs commented Jul 2, 2020

@houqp sorry for the late response. I moved the lint job to the macos build to spare some CI resource and also changed to check all crates.

@kszucs kszucs changed the title ARROW-9192: [Rust] run clippy to lint arrow crate in CI ARROW-9192: [CI][Rust] Add support for running clippy Jul 2, 2020
@kszucs
Copy link
Member

kszucs commented Jul 2, 2020

Created another follow-up to enable more clippy rules: ARROW-9296: [CI][Rust] Enable more clippy lint checks

Copy link
Member

@kszucs kszucs left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Merging on green.

@kszucs kszucs closed this in de6304f Jul 2, 2020
@houqp houqp deleted the ARROW-9192 branch July 2, 2020 19:04
@houqp
Copy link
Member Author

houqp commented Jul 2, 2020

Thanks @kszucs , at the mean time, I will try to fix more linting errors and see if we can just run clippy as is without the custom linting script before you start adding it to Archery.

sgnkc pushed a commit to sgnkc/arrow that referenced this pull request Jul 6, 2020
The goal of this PR is not to fix all the clippy errors/warnings in arrow crate, but to make sure no new linting errors are introduced in the future so we can slowly fix the remaining errors.

Closes apache#7501 from houqp/ARROW-9192

Lead-authored-by: Qingping Hou <dave2008713@gmail.com>
Co-authored-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants