Skip to content

Conversation

ricardozanini
Copy link
Member

Signed-off-by: Ricardo Zanini zanini@redhat.com

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:
Since the most used log framework is slf4j, and we are using it in our project, it doesn't make sense to bring another logging framework to our dependency list. One of our dependencies uses commons-logging. I just replaced it with a bridge to slf4j. So now, everything is logged using the same framework.

Signed-off-by: Ricardo Zanini <zanini@redhat.com>
@ricardozanini ricardozanini requested a review from tsurdilo as a code owner March 16, 2021 21:16
@@ -18,6 +18,10 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this depends needed?
You could just exclude commons logging like you do below
and have the client add whatever they might need

Copy link
Member Author

Choose a reason for hiding this comment

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

It's needed to give our transitive dependency the ability to log. Since they use commons-logging, it's likely that they require its interface. slf4j bridge does that: http://www.slf4j.org/legacy.html

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@tsurdilo tsurdilo merged commit 1defc45 into serverlessworkflow:main Mar 17, 2021
@ricardozanini ricardozanini deleted the remove-common-logging branch March 17, 2021 19:26
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.

2 participants