Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
lhotari commented on PR #22607: URL: https://github.com/apache/pulsar/pull/22607#issuecomment-2093242915 > Based on the review results, let's continue to use compile scope. When using the pulsar dependency, the downstream users must unify the slf4j version in the project. @nodece makes sense. slf4j 2.0.x comes with a BOM, so aligning slf4j versions is easy with it. I created PR #22646 for Pulsar. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
nodece commented on PR #22607: URL: https://github.com/apache/pulsar/pull/22607#issuecomment-2093193496 Based on the review results, let's continue to use compile scope. When using the pulsar dependency, the downstream users must unify the slf4j version in the project. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
nodece closed pull request #22607: [fix][build] Switch slf4j scope to provided from compile URL: https://github.com/apache/pulsar/pull/22607 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
nodece commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1588611172
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
> @nodece please make this compile scope so that we can resolve this review
@lhotari Looks like we can close(not merge) this pr. No
changes are required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
lhotari commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1588143914
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
@nodece please make this compile scope so that we can resolve this review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
nodece commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1583308460
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
Another point is that #22391 will be released at 3.3.0. When users upgrade
to the pulsar version, they must pay attention to the version of slf4j. We need
to mention this in the release note.
> In the future, since there are both SLF4J 1.7.x and 2.x, any project would
have to use dependency management to select the desired version for that
project.
In the mailing list, I said:
Unifying SLF4J is quite difficult, many libraries include the slf4j 1.x or
2.x, and when the slf4j version is inconsistent, we still need to use
``.
This PR ensures that downstream users can use either slf4j 1. x or 2. x,
once the pulsar uses the slf4j 2.x
api(https://www.slf4j.org/manual.html#fluent), this will force users to upgrade
to slf4j 2.x.
For me, using scope `compile` is correct.
> All Java based projects will have to deal with this eventually so this
challenge isn't specific to Pulsar.
+1, just like what I did in #22391, unify the slf4j version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
lhotari commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1583197077
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
@nodece Please make the scope `compile` (it's default, so drop `provided`)
for `slf4j-api`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
lhotari commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1583197077
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
@nodece Please make the scope `compile` for `slf4j-api`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
lhotari commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1583195671
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
> I see your reason. But as the conclusion, did you mean all downstream
projects need to import the `slf4j-api` dependency explicitly to get it around?
In the future, since there are both SLF4J 1.7.x and 2.x, any project would
have to use dependency management to select the desired version for that
project. Obviously if a project is using 2.x features, it will require 2.x
version at runtime.
I think that the correct solution is to keep `slf4j-api` in `compile` scope.
It's simply a bad solution to ignore the dependency. For `slf4j-simple` and
`jcl-over-slf4j`, the correct scope is `provided` since they are runtime only
type of dependencies. The same argumentation doesn't apply to `slf4j-api`.
> We must avoid introducing any breaking change, or at least, document the
breaking change and how should users get it around.
My assumption is that slf4j-api is binary compatible for all 1.7.x
interfaces in 2.x. . However, it will be necessary to document that Pulsar has
switched to use slf4j 2.x . All Java based projects will have to deal with this
eventually so this challenge isn't specific to Pulsar.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
BewareMyPower commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1582731198
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
I see your reason. But as the conclusion, did you mean all downstream
projects need to import the `slf4j-api` dependency explicitly to get it around?
We must avoid introducing any breaking change, or at least, document the
breaking change and how should users get it around.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
lhotari commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1582686634
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
> The default `compile` scope breaks many downstream applications, as I've
said in https://lists.apache.org/thread/4omvl62k4jhntj3rsywp14zzgh1l3l9q
I agree that it's fine for all other dependencies, but not for `slf4j-api`.
please check the reasoning in
https://github.com/apache/pulsar/pull/22607#discussion_r158278
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
BewareMyPower commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1582592478
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
The default `compile` scope breaks many downstream applications, as I've
said in https://lists.apache.org/thread/4omvl62k4jhntj3rsywp14zzgh1l3l9q
With the `compile` scope, we should revert
https://github.com/apache/pulsar/pull/22391.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
BewareMyPower commented on PR #22607: URL: https://github.com/apache/pulsar/pull/22607#issuecomment-2081964237 It seems some tests failed even in the 5th rerun, could you take a look? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
shoothzj commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1582575529
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
I prefer `slf4j-api` keep compile scope too. We can make this update a minor
update. It's not a breaking change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
shoothzj commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1582575529
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
I prefer `slf4j-api` too. We can make this update a minor update. It's not a
breaking change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
nodece commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r1582572984
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
/cc @BewareMyPower
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
lhotari commented on code in PR #22607:
URL: https://github.com/apache/pulsar/pull/22607#discussion_r158278
##
pom.xml:
##
@@ -765,18 +765,21 @@ flexible messaging model and an intuitive client
API.
org.slf4j
slf4j-api
${slf4j.version}
+provided
Review Comment:
I think that `slf4j-api` should be kept in default (compile) scope. Users of
the library can use dependency management to enforce a specific version of
slf4j-api. Dependency resolution picks the "closest" dependency by default when
there are version conflicts. It's better to rely on this for slf4j-api.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [fix][build] Switch slf4j scope to provided from compile [pulsar]
nodece commented on PR #22607: URL: https://github.com/apache/pulsar/pull/22607#issuecomment-2081573945 /pulsarbot rerun-failure-checks -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
