-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Replace commons-lang:commons-lang with org.apache.commons:commons-lang3 #19229
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
❌ Gradle check result for 95b5c99: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 95b5c99: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Craig Perkins <[email protected]>
/** | ||
* Minimal shim to satisfy Azure Classic which expects commons-lang 2.x. | ||
* Delegates to commons-lang3. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on this? It creates a shim around StringUtils from commons-lang3 that spoofs that this is coming from commons-lang:commons-lang. The alternative is upgrading azure deps, but that is non-trivial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be the azure dependency this plugin is using? Which is from February 2016? Adding this StringUtils wrapper hardly seems like the biggest problem here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its a non-trivial update. How do you think the upgrade should be handled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was more leaning towards deprecating this plugin for future removal :)
It appears to be using the "classic" Azure APIs. I think the ideal solution would be to replace this plugin with a plugin that uses modern Azure APIs (assuming this is need for an Azure discovery plugin at all, of course).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would definitely support taking it out of the core repo and create a separate repo as #17246 proposed.
❌ Gradle check result for bb27409: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for bb27409: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for bb27409: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for bb27409: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for bb27409: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for bb27409: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 6c3c7f7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19229 +/- ##
============================================
+ Coverage 72.79% 72.93% +0.14%
- Complexity 69605 69722 +117
============================================
Files 5658 5659 +1
Lines 320079 320089 +10
Branches 46345 46345
============================================
+ Hits 232996 233462 +466
+ Misses 68230 67718 -512
- Partials 18853 18909 +56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Replace commons-lang:commons-lang with org.apache.commons:commons-lang3
Related Issues
commons-lang is not maintained and has moved to commons-lang3 per maven central: https://mvnrepository.com/artifact/commons-lang/commons-lang
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.