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.

Sign up for GitHub

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

Jump to bottom

Reduce usage of Apache Commons Lang 2 #3409

Merged
merged 13 commits into from
Aug 14, 2024
Merged

Reduce usage of Apache Commons Lang 2 #3409

merged 13 commits into from
Aug 14, 2024

Conversation

cziegeler
Copy link
Contributor

Replace usage of Apache Commons Lang 2 with Apache Commons Lang 3
As noted in #3408 there are two places remaining (MailTemplate) which can be replaced as soon as there is an alternative.
This fixes #3408

@codecov Codecov
Copy link

codecov bot commented Aug 9, 2024

Codecov Report

Attention: Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 55.55%. Comparing base (7676ed5) to head (be4c599).
Report is 24 commits behind head on master.

Files Patch % Lines
...ommons/audit_log_search/AuditLogSearchRequest.java 50.00% 2 Missing ⚠️
.../adobe/acs/commons/reports/api/ReportExecutor.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master    #3409      +/-   ##
============================================
+ Coverage     55.34%   55.55%   +0.20%     
- Complexity     5543     5582      +39     
============================================
  Files           728      728              
  Lines         29705    29792      +87     
  Branches       3875     3886      +11     
============================================
+ Hits          16440    16550     +110     
+ Misses        11722    11689      -33     
- Partials       1543     1553      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cziegeler
Copy link
Contributor Author

@kwin You were right, we should convert the on-prem bundle into a fragment; I did this as part of this PR as we will most likely need this once an alternative for the MailTemplate is available.

@davidjgonzalez davidjgonzalez requested a review from kwin August 13, 2024 18:52
@cziegeler
Copy link
Contributor Author

@kwin Do you think we can already apply this? The remaining usage will be replaced once an alternative API is available for Cloud Service

kwin
kwin approved these changes Aug 14, 2024
View reviewed changes
Copy link
Contributor

@kwin kwin left a comment

Choose a reason for hiding this comment

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

Looks good except for one small glitch in the Maven name.

bundle-onprem/pom.xml Outdated
@@ -33,13 +33,18 @@
<!-- ====================================================================== -->

<artifactId>acs-aem-commons-bundle-onprem</artifactId>
<name>ACS AEM Commons Bundle - On Prem</name>
<description>Core ACS AEM Commons OSGi Bundle for AEM on prem only</description>
<name>ACS AEM Commons - Bundle Fragment (for On Prem</name>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing a closing parenthesis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for spotting, fixed now

@kwin kwin merged commit 9445753 into Adobe-Consulting-Services:master Aug 14, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwin kwin kwin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reduce usage of Apache Commons Lang 2
2 participants
@cziegeler @kwin