Skip to content

Conversation

@CharlesDuboisSAP
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP commented May 19, 2025

Context

#807

We previously used commons-configuration but the new version is at commons-configuration2

Feature scope:

  • Migrate to commons-configuration2

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Documentation updated
  • Release notes updated

@CharlesDuboisSAP CharlesDuboisSAP changed the title chore: Migrate to commons-configuration2 chore: Migrate to commons-configuration2 May 19, 2025
@rpanackal rpanackal added the please review Request to review a pull request label May 21, 2025
@newtork newtork changed the title chore: Migrate to commons-configuration2 chore: Remove vulnerable commons-configuration May 22, 2025
Copy link
Contributor Author

@CharlesDuboisSAP CharlesDuboisSAP left a comment

Choose a reason for hiding this comment

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

Please remove the dependency from the parent pom

@@ -1,3 +1,4 @@

Copy link
Contributor Author

Choose a reason for hiding this comment

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

breaking change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not consider new-line as breaking in context of properties file

Copy link
Contributor

@newtork newtork May 22, 2025

Choose a reason for hiding this comment

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

However, I've noticed this too.
When I added trim() to the String that was written to file to get rid of the new-line, I got hundreds of file-changes :(

@newtork newtork enabled auto-merge (squash) May 22, 2025 14:59
Copy link
Contributor

@newtork newtork left a comment

Choose a reason for hiding this comment

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

Jonas approved via second PR.
Charles also approved via slack :)

@newtork newtork merged commit 267e5d0 into main May 22, 2025
14 checks passed
@newtork newtork deleted the commons-configuration2 branch May 22, 2025 15:00
@newtork newtork mentioned this pull request Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

please review Request to review a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants