You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "timveil (via GitHub)" <gi...@apache.org> on 2023/11/01 16:08:49 UTC
[PR] Removing direct dependencies on `commons-logging` and replacing with `jcl-over-slf4j` [pinot]
timveil opened a new pull request, #11920:
URL: https://github.com/apache/pinot/pull/11920
This PR works to remove all direct and transitive dependencies on `commons-logging` due to well documented class-loading issues and lack of active development. In places where `commons-logging` was directly referenced, the dependency was replaced by `jcl-over-slf4j` to maintain compatability.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Removing direct dependencies on `commons-logging` and replacing with `jcl-over-slf4j` [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #11920:
URL: https://github.com/apache/pinot/pull/11920#issuecomment-1789386714
This is great! Can you also doublecheck if the new built repo doesn't have `commons-logging` pulled (check `.m2` folder)
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Removing direct dependencies on `commons-logging` and replacing with `jcl-over-slf4j` [pinot]
Posted by "timveil (via GitHub)" <gi...@apache.org>.
timveil commented on PR #11920:
URL: https://github.com/apache/pinot/pull/11920#issuecomment-1789437338
> This is great! Can you also doublecheck if the new built repo doesn't have `commons-logging` pulled (check `.m2` folder)
what repo are you referring to?
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Removing direct dependencies on `commons-logging` and replacing with `jcl-over-slf4j` [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #11920:
URL: https://github.com/apache/pinot/pull/11920#issuecomment-1789615269
> what repo are you referring to?
Sorry for the confusion. I think the intention for this PR is to remove `commons-logging` dependency completely and replace it with `jcl-over-slf4j`. Can you verify if this is the case by building the pinot project, and check if it doesn't pull `commons-logging` from maven?
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Removing direct dependencies on `commons-logging` and replacing with `jcl-over-slf4j` [pinot]
Posted by "timveil (via GitHub)" <gi...@apache.org>.
timveil commented on PR #11920:
URL: https://github.com/apache/pinot/pull/11920#issuecomment-1789844225
> > what repo are you referring to?
>
> Sorry for the confusion. I think the intention for this PR is to remove `commons-logging` dependency completely and replace it with `jcl-over-slf4j`. Can you verify if this is the case by building the pinot project, and check if it doesn't pull `commons-logging` from maven?
Yes I was able to verify this using IntelliJ + maven. In Intellij, after the changes, `commons-logging` no longer appears in any module or as an "External Library". I also verified this by searching the effective pom and looking at the build output.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Removing direct dependencies on `commons-logging` and replacing with `jcl-over-slf4j` [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11920:
URL: https://github.com/apache/pinot/pull/11920
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Removing direct dependencies on `commons-logging` and replacing with `jcl-over-slf4j` [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11920:
URL: https://github.com/apache/pinot/pull/11920#issuecomment-1789349908
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11920?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11920](https://app.codecov.io/gh/apache/pinot/pull/11920?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (9b0b7d5) into [master](https://app.codecov.io/gh/apache/pinot/commit/8d49b112c203e6e6f46ee9f4142cfd1616fc6470?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (8d49b11) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #11920 +/- ##
============================================
+ Coverage 61.40% 61.43% +0.02%
- Complexity 1145 1147 +2
============================================
Files 2378 2378
Lines 128851 128851
Branches 19926 19926
============================================
+ Hits 79127 79161 +34
+ Misses 44002 43957 -45
- Partials 5722 5733 +11
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (ø)` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.37% <ø> (+26.63%)` | :arrow_up: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.31% <ø> (+0.02%)` | :arrow_up: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.39% <ø> (+<0.01%)` | :arrow_up: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.29% <ø> (+0.04%)` | :arrow_up: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.43% <ø> (+0.02%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.43% <ø> (+0.02%)` | :arrow_up: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.61% <ø> (-0.01%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/11920/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.67% <ø> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
[see 9 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11920/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org