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

CORE-16786: Upgrade CLI PF4J to 3.10 and SLF4J to 2.0.6 #4722

Merged

Conversation

josephzunigadaly
Copy link
Contributor

@josephzunigadaly josephzunigadaly commented Sep 27, 2023

This change upgrades PF4J to 3.10, the change log can be viewed here:
https://github.com/pf4j/pf4j/blob/master/CHANGELOG.md#3100---2023-09-06

One of the changes included in PF4J is an upgrade to SLF4J 2. This change uses SLF4J 2 for the CLI and SLF4J 1 for Corda itself.

Related CLI PR: corda/corda-cli-plugin-host#194

@josephzunigadaly josephzunigadaly self-assigned this Sep 27, 2023
@josephzunigadaly josephzunigadaly force-pushed the jzd/CORE-15310-Upgrade-SLF4J-from-1.7-to-2.x branch 2 times, most recently from 663d7b9 to bcb54d8 Compare September 28, 2023 08:40
@josephzunigadaly josephzunigadaly force-pushed the jzd/CORE-15310-Upgrade-SLF4J-from-1.7-to-2.x branch from bcb54d8 to 05e7b37 Compare September 28, 2023 09:16
@josephzunigadaly josephzunigadaly force-pushed the jzd/CORE-15310-Upgrade-SLF4J-from-1.7-to-2.x branch from 354c593 to e85f0f7 Compare September 28, 2023 13:45
@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Sep 28, 2023

Jenkins build for PR 4722 build 8

Build Successful:
Jar artifact version produced by this PR: 5.1.0.0-alpha-1695986911615
Helm chart version produced by this PR: 5.1.0-alpha.1695986911615
Helm chart pushed to: oci://corda-os-docker-dev.software.r3.com/helm-charts/pr-4722/corda

@josephzunigadaly josephzunigadaly marked this pull request as ready for review September 28, 2023 15:55
@josephzunigadaly josephzunigadaly requested review from a team as code owners September 28, 2023 15:55
vkolomeyko
vkolomeyko previously approved these changes Sep 28, 2023
Copy link
Contributor

@vkolomeyko vkolomeyko left a comment

Choose a reason for hiding this comment

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

LGTM

ronanbrowne
ronanbrowne previously approved these changes Sep 28, 2023
Copy link
Contributor

@ronanbrowne ronanbrowne left a comment

Choose a reason for hiding this comment

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

LGTM

YashNabar
YashNabar previously approved these changes Sep 29, 2023
Copy link
Contributor

@YashNabar YashNabar left a comment

Choose a reason for hiding this comment

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

LGTM (@corda/corda-platform-network-team files)

@simon-johnson-r3
Copy link
Contributor

Have we tested CorDapps built with Corda 5.0 still work after this change? Having had a conversation with Chris in the last couple of weeks, I would expect it does, however someone suggested to me they'd tried this and it didn't work because CorDapps were pinned to an SLF4J bundle version in their metadata (which I believe shouldn't be the case, but might have been broken).

@josephzunigadaly
Copy link
Contributor Author

Hello @simon-johnson-r3
The CorDapps run by the smoke tests are all running with SLF4J version 1. The API repo has a gradle constraint that prevents any CorDapp from accidentally upgrading to version 2.
From a CorDapp point of view, there is no change here. The only part that will use SLF4J version 2 is the CLI.

@josephzunigadaly josephzunigadaly changed the title CORE-15310: Upgrade CLI PF4J to 3.10 and SLF4J to 2.0.6 CORE-16786: Upgrade CLI PF4J to 3.10 and SLF4J to 2.0.6 Sep 29, 2023
Copy link
Contributor

@vkolomeyko vkolomeyko left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@YashNabar YashNabar left a comment

Choose a reason for hiding this comment

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

LGTM (@corda/corda-platform-network-team files)

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