Skip to content

[cleanup][misc] Fix imports using shaded dependencies#18113

Merged
nicoloboschi merged 1 commit into
apache:masterfrom
nahguam:guava-imports
Oct 22, 2022
Merged

[cleanup][misc] Fix imports using shaded dependencies#18113
nicoloboschi merged 1 commit into
apache:masterfrom
nahguam:guava-imports

Conversation

@nahguam

@nahguam nahguam commented Oct 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

Some usages of Guava and Awaitility are coming from the versions shaded in
Curator and Testcontainers respectively.

Modifications

  • Use the correct Guava imports.
  • Use the correct Awaitility imports.
  • Remove Maps.newHashMap due to modernizer-maven-plugin error

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: nahguam#3

@nahguam nahguam changed the title [cleanup][general] Use correct Guava imports [cleanup][misc] Use correct Guava imports Oct 19, 2022
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Oct 19, 2022
@Technoboy- Technoboy- added this to the 2.12.0 milestone Oct 19, 2022
@Technoboy- Technoboy- added the type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use label Oct 19, 2022
@nahguam nahguam changed the title [cleanup][misc] Use correct Guava imports [cleanup][misc] Fix imports using shaded dependencies Oct 21, 2022
nicoloboschi
nicoloboschi requested changes Oct 21, 2022
View reviewed changes

@nicoloboschi nicoloboschi 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.

Thanks! Could you add that package in the forbidden imports to avoid "regressions" ?

pulsar/buildtools/src/main/resources/pulsar/checkstyle.xml

Line 140 in 1d6824c

value="autovalue.shaded, avro.shaded, bk-shade, com.google.api.client.repackaged, com.google.appengine.repackaged" />

[cleanup][misc] Fix imports using shaded dependencies
f0c0b20 
## Motivation

Some usages of Guava and Awaitility are coming from the versions shaded in
Curator and Testcontainers respectively.

## Modifications

* Use the correct Guava imports.
* Use the correct Awaitility imports.
* Remove Maps.newHashMap due to modernizer-maven-plugin error
* Add shaded packages to checkstyle IllegalImports
@nahguam

nahguam commented Oct 21, 2022

Copy link
Copy Markdown
Contributor Author

Thanks! Could you add that package in the forbidden imports to avoid "regressions" ?

pulsar/buildtools/src/main/resources/pulsar/checkstyle.xml

Line 140 in 1d6824c

value="autovalue.shaded, avro.shaded, bk-shade, com.google.api.client.repackaged, com.google.appengine.repackaged" />

@nicoloboschi Nice! done :)

@codecov-commenter

codecov-commenter commented Oct 21, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.93%. Comparing base (6c65ca0) to head (f0c0b20).
⚠️ Report is 3529 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #18113       +/-   ##
=============================================
+ Coverage     34.91%   45.93%   +11.01%     
- Complexity     5707    17654    +11947     
=============================================
  Files           607     1574      +967     
  Lines         53396   128574    +75178     
  Branches       5712    14155     +8443     
=============================================
+ Hits          18644    59062    +40418     
- Misses        32119    63385    +31266     
- Partials       2633     6127     +3494
Flag Coverage Δ
unittests 45.93% <ø> (+11.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ache/pulsar/testclient/PerformanceTransaction.java 66.92% <ø> (ø)

... and 1118 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nicoloboschi

nicoloboschi commented Oct 21, 2022

Copy link
Copy Markdown
Contributor

/pulsarbot rerun-failure-checks

@nicoloboschi nicoloboschi merged commit 845d992 into apache:master Oct 22, 2022
@nahguam nahguam deleted the guava-imports branch November 9, 2022 08:46
@lhotari lhotari mentioned this pull request May 30, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Technoboy- Technoboy- Technoboy- approved these changes

@nodece nodece nodece approved these changes

@nicoloboschi nicoloboschi nicoloboschi approved these changes

Assignees

@nahguam nahguam

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@nahguam @codecov-commenter @nicoloboschi @Technoboy- @nodece