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.

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

[WFCORE-6664] Upgrade SLF4J from 2.0.9 to 2.0.11 #5831

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

yersan
Copy link
Collaborator

@yersan yersan commented Jan 15, 2024

@yersan yersan requested a review from jamezp January 15, 2024 10:30
@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Jan 15, 2024
Copy link
Member

@jamezp jamezp left a comment

Choose a reason for hiding this comment

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

I think this is okay. There is one commit that is not backwards compatible, but it's on a utility that should not be used. See qos-ch/slf4j@316b5d1 and qos-ch/slf4j#361

@yersan
Copy link
Collaborator Author

yersan commented Jan 18, 2024

@jamezp thanks for looking at this. You are talking about the Util.report() method removal, isn't it?

The class itself states that it is an internal utility class, so I think we are safe. The slf4j-api module is distributed in the modules directory, so potentially there could be user applications using it, but again, it states that it is an internal utility class, so if someone is using it, its usage should be removed. Looking at what the Report method does, its usage doesn't look useful.

@jamezp
Copy link
Member

jamezp commented Jan 18, 2024

@jamezp thanks for looking at this. You are talking about the Util.report() method removal, isn't it?

The class itself states that it is an internal utility class, so I think we are safe. The slf4j-api module is distributed in the modules directory, so potentially there could be user applications using it, but again, it states that it is an internal utility class, so if someone is using it, its usage should be removed. Looking at what the Report method does, its usage doesn't look useful.

@yersan Yes, that is what I was referring to. I think it's okay too, I just wanted to make sure to point out that we saw it and we think it's okay :)

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Jan 19, 2024
@yersan yersan merged commit 05ad2d1 into wildfly:main Jan 19, 2024
12 checks passed
@yersan yersan deleted the WFCORE-6664 branch January 19, 2024 16:03
@yersan
Copy link
Collaborator Author

yersan commented Jan 19, 2024

Thanks @jamezp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
2 participants