Skip to content

HIVE-22618: Fix checkstyle violations for ParseUtils - #4451

Merged
deniskuzZ merged 4 commits into
apache:masterfrom
okumin:HIVE-22618-checkstyle
Aug 29, 2023
Merged

HIVE-22618: Fix checkstyle violations for ParseUtils#4451
deniskuzZ merged 4 commits into
apache:masterfrom
okumin:HIVE-22618-checkstyle

Conversation

@okumin

@okumin okumin commented Jun 25, 2023

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR would just format ParseUtils.java.

https://issues.apache.org/jira/browse/HIVE-22618

I recommend reviewers take a look at the diff of this PR via the following URL.
https://github.com/apache/hive/compare/master...okumin:HIVE-22618-checkstyle?w=1

Why are the changes needed?

Indentations of this file are totally broken and I felt it is hard to modify the file while working on another issue.

I'd say we have no strong reasons to merge this one. I just created it because I hope other PRs are unlikely to conflict with this change now since it is not often updated recently.

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

No

How was this patch tested?

These are the original checkstyle report.

root@fcc249b8c79d:~/hive# mvn -pl ql checkstyle:check -Dcheckstyle.consoleOutput=true | grep 'parse/ParseUtils.java'
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:227:13: warning: '(' is followed by whitespace.
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:227:25: warning: ')' is preceded with whitespace.
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:228:15: warning: '(' is followed by whitespace.
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:228:39: warning: ')' is preceded with whitespace.
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:241: warning: 'if' child have incorrect indentation level 8, expected level should be 6.
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:242: warning: 'if rcurly' have incorrect indentation level 6, expected level should be 4.
...
(too many violations)
root@fcc249b8c79d:~/hive#
root@fcc249b8c79d:~/hive# mvn -pl ql checkstyle:check -Dcheckstyle.consoleOutput=true | grep 'parse/TestParseUtils.java'
/root/hive/ql/src/test/org/apache/hadoop/hive/ql/parse/TestParseUtils.java:123:75: warning: ',' is not followed by whitespace.
/root/hive/ql/src/test/org/apache/hadoop/hive/ql/parse/TestParseUtils.java:129:75: warning: ',' is not followed by whitespace.
/root/hive/ql/src/test/org/apache/hadoop/hive/ql/parse/TestParseUtils.java:136:75: warning: ',' is not followed by whitespace.
/root/hive/ql/src/test/org/apache/hadoop/hive/ql/parse/TestParseUtils.java:142:75: warning: ',' is not followed by whitespace.
root@fcc249b8c79d:~/hive# 

This is the result after applying this change. I'm keeping the TODO ones since it looks intentional.

root@fcc249b8c79d:~/hive# mvn -pl ql checkstyle:check -Dcheckstyle.consoleOutput=true | grep 'parse/ParseUtils.java'
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:426: warning: Comment matches to-do format 'TODO:'.
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:460: warning: Comment matches to-do format 'TODO:'.
/root/hive/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java:488: warning: Comment matches to-do format 'TODO:'.
root@fcc249b8c79d:~/hive#
root@fcc249b8c79d:~/hive# mvn -pl ql checkstyle:check -Dcheckstyle.consoleOutput=true | grep 'parse/TestParseUtils.java'
root@fcc249b8c79d:~/hive#

@okumin okumin changed the title Hive 22618 checkstyle HIVE-22618: Fix checkstyle violations for ParseUtils Jun 25, 2023
@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@aturoczy aturoczy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

@aturoczy

Copy link
Copy Markdown
Contributor

@ayushtkn @zabetak @deniskuzZ Could you pls check? This change are just code style change. I think it should be OK.

@deniskuzZ

Copy link
Copy Markdown
Member

can't we do the reformat when we actually change something in that class? i don't see any benefit at this point

@okumin

okumin commented Aug 25, 2023

Copy link
Copy Markdown
Contributor Author

@deniskuzZ Thanks for taking a look! If I remember correctly, I filed this ticket paired with HIVE-22601 to resolve a parser issue. I found that another ticket has resolved the problem of HIVE-22601, so I created this PR just to get the cosmetic part closed. It is totally OK for me to close this PR and void HIVE-22618. My purpose is to close the orphaned ticket assigned to me.

deniskuzZ

This comment was marked as outdated.

@deniskuzZ
deniskuzZ dismissed their stale review August 29, 2023 10:59

just refactor

@deniskuzZ deniskuzZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@deniskuzZ
deniskuzZ merged commit 26ed33a into apache:master Aug 29, 2023
@okumin
okumin deleted the HIVE-22618-checkstyle branch August 29, 2023 12:39
@okumin

okumin commented Aug 29, 2023

Copy link
Copy Markdown
Contributor Author

Thanks!

scarlin-cloudera pushed a commit to scarlin-cloudera/hive that referenced this pull request Aug 29, 2023
tarak271 pushed a commit to tarak271/hive-1 that referenced this pull request Dec 19, 2023
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