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

ALL:fixing static analysis warnings #4376

Closed
wants to merge 54 commits into from

Conversation

felixYyu
Copy link
Contributor

@felixYyu felixYyu commented Mar 22, 2022

this PR fixed these grouping parentheses are unnecessary. cc @rdblue @RussellSpitzer

iceberg\core\src\main\java\org\apache\iceberg\BaseFileScanTask.java:224: [UnnecessaryParentheses] These grouping parentheses are unnecessary; it is unlikely the code will be misinterpreted without them
          (this.file().equals(other.file())) &&
          ^
    (see https://errorprone.info/bugpattern/UnnecessaryParentheses)
  Did you mean 'this.file().equals(other.file()) &&'?

@github-actions github-actions bot added the hive label Mar 25, 2022
@felixYyu felixYyu marked this pull request as draft March 28, 2022 05:57
@felixYyu felixYyu marked this pull request as ready for review March 28, 2022 05:57
@felixYyu
Copy link
Contributor Author

please review again,thanks @RussellSpitzer

Copy link
Member

@RussellSpitzer RussellSpitzer left a comment

Choose a reason for hiding this comment

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

Most of these look fine, I would if possible (and I know I asked to group these together) separate out the changes which make semantic changes to the code into their own PR's.

For example

"Removing an initialization that sets a variable to null" Good, keep here doesn't actually change anything
"Removing a fully defined type that is already imported" good keep here
"Supressions we know are false positive" Keep here
"Removing empty java docs, fixing java docs" Keep here
"Changing parens" Keep here

--

"Changes an iterator to a list" Separate PR
"Changing the access modifiers on all the JDBC Static Values" Separate PR
"Changes in date handling classes" Separate PR

@felixYyu
Copy link
Contributor Author

I will separate out the changes which make semantic changes to the code into their own PR's.

@felixYyu
Copy link
Contributor Author

felixYyu commented Mar 31, 2022

The task was not fully completed,It will be finished today. @RussellSpitzer

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.

3 participants