Skip to content

HDDS-3341. Checkstyle fails for new modules/versions #768

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

Merged
merged 1 commit into from
Apr 3, 2020

Conversation

adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Make checkstyle.sh work with new modules/versions:

  1. first attempt to run the check only
  2. if it fails due to anything other than style violation, try to compile sources, too.

Ignore violations on Protobuf generated sources.

Output from the first attempt is only printed with some delay only if and when it completes successfully, to avoid duplicate Maven logs. This may be a bit annoying when run manually. Please let me know if duplicated but immediate output is preferred.

https://issues.apache.org/jira/browse/HDDS-3341

How was this patch tested?

  1. Tested on HDDS-3312, which introduces a new hadoop-hdds-hadoop-dependency-client module and fails with Could not find artifact org.apache.hadoop:hadoop-hdds-hadoop-dependency-client:jar:0.6.0-SNAPSHOT in apache.snapshots.https. With this change the check passes:
$ hadoop-ozone/dev-support/checks/checkstyle.sh
...
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  02:55 min
...
$ echo $?
0
  1. Tested on master, where all modules are available, so no compilation is needed:
$ hadoop-ozone/dev-support/checks/checkstyle.sh
...
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  14.093 s
...
$ echo $?
0
  1. Introduced an unused import, verified that violation is reported:
$ hadoop-ozone/dev-support/checks/checkstyle.sh
...
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  6.226 s
...
hadoop-ozone/common/src/main/java/org/apache/hadoop/hdds/protocol/StorageType.java
 20: Unused import - java.util.List.
$ echo $?
1

https://github.com/adoroszlai/hadoop-ozone/runs/558999926

Copy link
Contributor

@dineshchitlangia dineshchitlangia left a comment

Choose a reason for hiding this comment

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

+1 @adoroszlai . I verified this patch in my local setup and tested with different combination of checkstyle violations too. It works as desired.

Merging this to master, thanks for the improvement.

@dineshchitlangia dineshchitlangia merged commit 11b0984 into apache:master Apr 3, 2020
@adoroszlai adoroszlai deleted the HDDS-3341 branch April 4, 2020 06:20
@adoroszlai
Copy link
Contributor Author

Thanks @dineshchitlangia for reviewing and merging this.

@elek
Copy link
Member

elek commented Apr 4, 2020

Good trick.

The related problem is that our daily build is failing on the apache jenkins (which did the deploy) and our github daily build doesn't have permission to upload to the apache snapshot repository. We might need to ask INFRA to set a secret to make it possible to deploy. (Which would make the original problem more rare)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants