Skip to content

ARROW-11602: [Rust] Clippy lints - #9475

Closed
nevi-me wants to merge 1 commit into
apache:masterfrom
nevi-me:2021-feb-clippy
Closed

ARROW-11602: [Rust] Clippy lints#9475
nevi-me wants to merge 1 commit into
apache:masterfrom
nevi-me:2021-feb-clippy

Conversation

@nevi-me

Copy link
Copy Markdown
Contributor

There's some clippy lints that are returning errors, where they haven't before. Not sure what's happening, as we've pinned versions.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@alamb

Copy link
Copy Markdown
Contributor

Thank you @nevi-me -- looks like there are still a few more lints being hit

@alamb

alamb commented Feb 11, 2021

Copy link
Copy Markdown
Contributor

Looks to me like Rust 1.50 was released today: https://forge.rust-lang.org/

Screen Shot 2021-02-11 at 2 42 24 PM

So clippy got pickier perhaps
https://blog.rust-lang.org/2021/02/11/Rust-1.50.0.html

@nevi-me

Copy link
Copy Markdown
ContributorAuthor

darn it, I was hoping it isn't a new stable release, because we have a lot of unnecessary Result<()>. This will take me longer

@alamb

Copy link
Copy Markdown
Contributor

@nevi-me -- I am working on a fix for the other lint errors based on this PR. My plan is to just disable the lints that are still failing -- because as you say changing Result<..> signatures is a much larger change

@nevi-me

Copy link
Copy Markdown
ContributorAuthor

Thanks @alamb, it's inefficient use of time to try pursue all the lints now. Thanks, I'm closing this.

@nevi-menevi-me closed this Feb 11, 2021
@alambalamb changed the title [Rust] Clippy lintsARROW-11602: [Rust] Clippy lintsFeb 11, 2021
@github-actions

Copy link
Copy Markdown

@alamb

Copy link
Copy Markdown
Contributor

#9476

jorgecarleitao pushed a commit that referenced this pull request Feb 11, 2021
# Rationale:
CI uses "stable" rust. 1.50 stable was released today: https://blog.rust-lang.org/2021/02/11/Rust-1.50.0.html
The new clippy is pickier resulting in many clippy warnings such as https://github.com/apache/arrow/pull/9469/checks?check_run_id=1881854256
We need to get CI back green
# Changes
Based on based on #9475 from @nevi-me, this PR aims to get the CI green as soon as possible.
Ideally, we would fix the actual lint problems, However, when I tried to do so the lints propagated into a significant change -- as clippy says to remove the `Result` but then there are a bunch of call sites that then also need t be changed
I want to get CI back clean as soon as possible so I just hammered through and cleaned up as best I could as well as sprinkling in various `#[allow(clippy::unnecessary_wraps)]` as necessary to get a clean run.
My rationale is that the code is no worse than it was before. Though it could be better!
Closes#9476 from alamb/ARROW-11602-lints
Authored-by: Neville Dipale <nevilledips@gmail.com>
Signed-off-by: Jorge C. Leitao <jorgecarleitao@gmail.com>
sgnkc pushed a commit to sgnkc/arrow that referenced this pull request Feb 17, 2021
# Rationale:
CI uses "stable" rust. 1.50 stable was released today: https://blog.rust-lang.org/2021/02/11/Rust-1.50.0.html
The new clippy is pickier resulting in many clippy warnings such as https://github.com/apache/arrow/pull/9469/checks?check_run_id=1881854256
We need to get CI back green
# Changes
Based on based on apache#9475 from @nevi-me, this PR aims to get the CI green as soon as possible.
Ideally, we would fix the actual lint problems, However, when I tried to do so the lints propagated into a significant change -- as clippy says to remove the `Result` but then there are a bunch of call sites that then also need t be changed
I want to get CI back clean as soon as possible so I just hammered through and cleaned up as best I could as well as sprinkling in various `#[allow(clippy::unnecessary_wraps)]` as necessary to get a clean run.
My rationale is that the code is no worse than it was before. Though it could be better!
Closesapache#9476 from alamb/ARROW-11602-lints
Authored-by: Neville Dipale <nevilledips@gmail.com>
Signed-off-by: Jorge C. Leitao <jorgecarleitao@gmail.com>
alamb pushed a commit to apache/arrow-rs that referenced this pull request Apr 20, 2021
# Rationale:
CI uses "stable" rust. 1.50 stable was released today: https://blog.rust-lang.org/2021/02/11/Rust-1.50.0.html
The new clippy is pickier resulting in many clippy warnings such as https://github.com/apache/arrow/pull/9469/checks?check_run_id=1881854256
We need to get CI back green
# Changes
Based on based on apache/arrow#9475 from @nevi-me, this PR aims to get the CI green as soon as possible.
Ideally, we would fix the actual lint problems, However, when I tried to do so the lints propagated into a significant change -- as clippy says to remove the `Result` but then there are a bunch of call sites that then also need t be changed
I want to get CI back clean as soon as possible so I just hammered through and cleaned up as best I could as well as sprinkling in various `#[allow(clippy::unnecessary_wraps)]` as necessary to get a clean run.
My rationale is that the code is no worse than it was before. Though it could be better!
Closes#9476 from alamb/ARROW-11602-lints
Authored-by: Neville Dipale <nevilledips@gmail.com>
Signed-off-by: Jorge C. Leitao <jorgecarleitao@gmail.com>
Sign up for freeto 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.

2 participants

@nevi-me@alamb