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.

Sign up for GitHub

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

Jump to bottom

[ISSUE 3891] Optimize log4j dependency in Bookkeeper #3892

Merged
merged 1 commit into from
Mar 28, 2023
Merged

[ISSUE 3891] Optimize log4j dependency in Bookkeeper #3892

merged 1 commit into from
Mar 28, 2023

Conversation

shoothzj
Copy link
Member

Motivation

The current parent pom in Bookkeeper includes log4j-core and slf4j-log4j-impl dependencies, which are being transitively passed down to many modules. This has resulted in an unnecessary increase in the number of dependencies for these modules.

Changes

  • move the log4j-impl and log4j-core to testtools
  • declare log4j-impl and log4j-core dependency in bookkeeper server module.

Fix #3891

Also see

@shoothzj shoothzj mentioned this pull request Mar 27, 2023
Merged
15 tasks
@shoothzj
Copy link
Member Author

shoothzj commented Mar 27, 2023 •
edited
Loading

I verified the module doesn't contains log4j dependency. And the server package contains it.
Thanks for @tisonkun nice catch.

@hangc0276 hangc0276 added dependencies Pull requests that update a dependency file release/4.15.5 labels Mar 28, 2023
@hangc0276 hangc0276 added this to the 4.16.0 milestone Mar 28, 2023
@hangc0276
Copy link
Contributor

I will trigger a new RC for 4.16.0 to include this PR.

@tisonkun
Copy link
Member

Thanks for taking care of this!

horizonzy
horizonzy approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@horizonzy horizonzy left a comment

Choose a reason for hiding this comment

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

LGTM

eolivelli
eolivelli approved these changes Mar 28, 2023
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

@eolivelli eolivelli merged commit e440132 into apache:master Mar 28, 2023
@shoothzj shoothzj deleted the optimize-log4j branch March 28, 2023 09:04
hangc0276 pushed a commit that referenced this pull request Mar 29, 2023
@shoothzj @hangc0276
Optimize log4j dependency in Bookkeeper (#3892)
378eb07 
(cherry picked from commit e440132)
zymap pushed a commit that referenced this pull request Dec 6, 2023
@shoothzj @zymap
Optimize log4j dependency in Bookkeeper (#3892)
d3e2f5a 
(cherry picked from commit e440132)
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
@shoothzj
Optimize log4j dependency in Bookkeeper (apache#3892)
b9434be
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hangc0276 hangc0276 hangc0276 approved these changes

@eolivelli eolivelli eolivelli approved these changes

@tisonkun tisonkun tisonkun approved these changes

@horizonzy horizonzy horizonzy approved these changes

@merlimat merlimat Awaiting requested review from merlimat

@zymap zymap Awaiting requested review from zymap

Assignees

@shoothzj shoothzj

Labels
cherry-picked/branch-4.15 dependencies Pull requests that update a dependency file release/4.15.5
Projects
None yet
Milestone
4.16.0
Development

Successfully merging this pull request may close these issues.

BookKeeper's parent pom.xml should not pull log4j implementations
6 participants
@shoothzj @hangc0276 @tisonkun @eolivelli @horizonzy @zymap