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

Refactor log4j-core and log4j-api dependencies out of root POM #24605

Merged
merged 1 commit into from
Mar 4, 2025
Merged

Refactor log4j-core and log4j-api dependencies out of root POM #24605

merged 1 commit into from
Mar 4, 2025
+6 −16

Conversation

Dilli-Babu-Godari
Copy link
Contributor

@Dilli-Babu-Godari Dilli-Babu-Godari commented Feb 21, 2025 •
edited
Loading

Description

revert of PR: #24507

Upgrading module specific dependencies.

Motivation and Context

image

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
*  refactored org.apache.logging.log4j:log4j-core out of root POM
*  refactored org.apache.logging.log4j:log4j-api out if root POM

@Dilli-Babu-Godari Dilli-Babu-Godari requested a review from a team as a code owner February 21, 2025 08:56
@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Feb 21, 2025
@prestodb-ci prestodb-ci requested a review from a team February 21, 2025 08:56
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Feb 21, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: Dilli-Babu-Godari (7a3d0ca)

@prestodb-ci prestodb-ci requested review from zuyu and BryanCutler and removed request for a team February 21, 2025 08:56
@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the revert_log4j_bump branch 5 times, most recently from 49d37f6 to 217ab12 Compare February 23, 2025 12:40
@Dilli-Babu-Godari Dilli-Babu-Godari assigned aaneja and unassigned aaneja Feb 24, 2025
@Dilli-Babu-Godari Dilli-Babu-Godari mentioned this pull request Feb 24, 2025
Merged
6 tasks
@BryanCutler
Copy link
Contributor

One minor nit: Your commit message still says "Upgrade log4j-core and log4j-api dependencies", could you update to indicate you are reverting and describe why?

imjalpreet
imjalpreet approved these changes Feb 25, 2025
View reviewed changes
Copy link
Member

@imjalpreet imjalpreet left a comment

Choose a reason for hiding this comment

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

@BryanCutler I agree commit message can be updated but it's more of a refactor than a revert.

I think we can update the commit message to say something like Refactor log4j-core and log4j-api dependencies out of root POM

@Dilli-Babu-Godari
Refactor log4j-core and log4j-api dependencies out of root POM
7a3d0ca 
refactor of PR: prestodb#24507

Upgrading module specific dependencies.

Upgraded org.apache.logging.log4j:log4j-core from 2.17.1 to 2.24.3
Upgraded org.apache.logging.log4j:log4j-api from 2.17.1 to 2.24.3
@Dilli-Babu-Godari
Copy link
Contributor Author

@BryanCutler @imjalpreet Can we merge this PR ?

@imjalpreet
Copy link
Member

@jaystarshot could you please have a look whenever you get a chance? Thanks!

@jaystarshot
Copy link
Member

Don't think the release note is correct. Also please update the description since this is not a revert

@Dilli-Babu-Godari Dilli-Babu-Godari changed the title Upgrade log4j-core and log4j-api dependencies Refactor log4j-core and log4j-api dependencies out of root POM Feb 28, 2025
@Dilli-Babu-Godari
Copy link
Contributor Author

Dilli-Babu-Godari commented Feb 28, 2025 •
edited
Loading

Don't think the release note is correct. Also please update the description since this is not a revert

@jaystarshot I have made the changes. could you please take a look once again ?

@jaystarshot jaystarshot merged commit 55a935b into prestodb:master Mar 4, 2025
53 checks passed
This was referenced Mar 10, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaneja aaneja aaneja approved these changes

@BryanCutler BryanCutler BryanCutler approved these changes

@imjalpreet imjalpreet imjalpreet approved these changes

@jaystarshot jaystarshot jaystarshot approved these changes

@zuyu zuyu Awaiting requested review from zuyu zuyu was automatically assigned from prestodb/ibm-reviewers

Assignees
No one assigned
Labels
from:IBM PR from IBM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Dilli-Babu-Godari @BryanCutler @imjalpreet @jaystarshot @aaneja @prestodb-ci