Skip to content

[GOBBLIN-1480] Cleanup unused imports and enable checkstyle for them#3320

Merged
autumnust merged 1 commit intoapache:masterfrom
aplex:checkstyle-import-fixes
Jul 1, 2021
Merged

[GOBBLIN-1480] Cleanup unused imports and enable checkstyle for them#3320
autumnust merged 1 commit intoapache:masterfrom
aplex:checkstyle-import-fixes

Conversation

@aplex
Copy link
Contributor

@aplex aplex commented Jun 23, 2021

Previously, checkstyle was configured to check only a small subset of
things, and imports were not one of them. As a result, reviewers spend
their time commenting on code style, when it can be fully verified
automatically by CI job.

We enable unused/redundant import checks, and fix existing issues, so
that the build could complete successfully.

The only relevant change is in checkstyle.xml, all other files where
changed by IntelliJ "optimize imports" function. For most files it
just removed unused imports, but some of them had imports incorrectly
ordered, and IntelliJ fixed that as well.

https://issues.apache.org/jira/browse/GOBBLIN-1480

@aplex aplex force-pushed the checkstyle-import-fixes branch from 575e5a3 to 9044b4c Compare June 23, 2021 20:41
Copy link
Contributor

@arjun4084346 arjun4084346 left a comment

Choose a reason for hiding this comment

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

Are you planning to add rest of the checkstyle configurations in another PR?

@codecov-commenter
Copy link

codecov-commenter commented Jun 23, 2021

Codecov Report

Merging #3320 (39b2dea) into master (bf37c76) will decrease coverage by 3.66%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3320      +/-   ##
============================================
- Coverage     46.50%   42.83%   -3.67%     
+ Complexity    10041     1931    -8110     
============================================
  Files          2041      394    -1647     
  Lines         79349    16870   -62479     
  Branches       8845     2072    -6773     
============================================
- Hits          36898     7226   -29672     
+ Misses        39020     8846   -30174     
+ Partials       3431      798    -2633     
Impacted Files Coverage Δ
...n/java/org/apache/gobblin/cli/CliTablePrinter.java 0.00% <ø> (ø)
...a/org/apache/gobblin/aws/GobblinAWSTaskRunner.java 0.00% <ø> (ø)
.../gobblin/cluster/GobblinClusterMetricTagNames.java 0.00% <ø> (ø)
...pache/gobblin/cluster/GobblinHelixTaskMetrics.java 97.61% <ø> (ø)
...he/gobblin/cluster/TaskRunnerSuiteThreadModel.java 100.00% <ø> (ø)
...lin/cluster/event/CancelJobConfigArrivalEvent.java 0.00% <ø> (ø)
.../store/hdfs/SimpleLocalHDFSConfigStoreFactory.java 60.00% <ø> (ø)
...e/gobblin/example/hadoop/HadoopTextFileSource.java 0.00% <ø> (ø)
...che/gobblin/metastore/DatabaseJobHistoryStore.java 78.26% <ø> (ø)
...obblin/metastore/FileContextBasedFsStateStore.java 0.00% <ø> (ø)
... and 1547 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e03329...39b2dea. Read the comment docs.

@aplex aplex force-pushed the checkstyle-import-fixes branch 3 times, most recently from 39b2dea to c06d419 Compare July 1, 2021 19:49
Previously, checkstyle was configured to check only a small subset of
things, and imports were not one of them. As a result, reviewers spend
their time commenting on code style, when it can be fully verified
automatically by CI job.

We enable unused/redundant import checks, and fix existing issues, so
that the build could complete successfully.

The only relevant change is in checkstyle.xml, all other files where
changed by IntelliJ "optimize imports" function. For most files it
just removed unused imports, but some of them had imports incorrectly
ordered, and IntelliJ fixed that as well.
@aplex aplex force-pushed the checkstyle-import-fixes branch from c06d419 to 7491c2d Compare July 1, 2021 19:53
@sv2000
Copy link
Contributor

sv2000 commented Jul 1, 2021

@aplex Thanks for this clean up PR!

Copy link
Contributor

@sv2000 sv2000 left a comment

Choose a reason for hiding this comment

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

+1.

@autumnust autumnust merged commit 5b495af into apache:master Jul 1, 2021
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.

5 participants