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.

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

Move opencsv to be non-shaded #2903

Merged
merged 2 commits into from
Jun 8, 2022
Merged

Move opencsv to be non-shaded #2903

merged 2 commits into from
Jun 8, 2022

Conversation

nvoxland
Copy link
Contributor

@nvoxland nvoxland commented Jun 2, 2022

Description

Rather than shading opencsv into the liquibase-core.jar, define it as a dependency in the shipped pom.xml and include it as a separate jar in the internal/lib directory.

Opencsv has dependencies on commons-collections4, commons-lang3, and commons-text so they are will be in the internal/lib directory as well.

Partly addresses #2852 but not fully so not marking it as "fixes"

Expected changes

  • No liquibase/repackaged directory in liquibase-core.jar anymore
  • commons-collections4.jar added to to internal/lib
  • commons-lang3.jar added to to internal/lib
  • commons-text.jar added to to internal/lib
  • opencsv.jar added to internal/lib
  • Nothing else new added to internal/lib

Testing

The CLI and maven plugin should continue to handle changelogs with loadData pointing to CSV files with no issues.

@nvoxland nvoxland added this to To Do in Conditioning++ via automation Jun 2, 2022
@github-actions
Copy link

github-actions bot commented Jun 2, 2022

Unit Test Results

  4 524 files  ±0    4 524 suites  ±0   32m 28s ⏱️ -22s
  4 422 tests ±0    4 208 ✔️ ±0     214 💤 ±0  0 ±0 
52 344 runs  ±0  47 336 ✔️ ±0  5 008 💤 ±0  0 ±0 

Results for commit c84b7fa. ± Comparison against base commit c1a67a7.

@FBurguer FBurguer self-assigned this Jun 8, 2022
@FBurguer
Copy link

FBurguer commented Jun 8, 2022

There is still a liquibase/repackaged dir in liquibase-core.jar, it has nothing to do with opencsv though, so thats fine.
The shipped pom file has the correct dependency and all 4 of the needed jar file are shipped in internal/lib as well.

@nvoxland nvoxland merged commit b3249c3 into master Jun 8, 2022
Conditioning++ automation moved this from To Do to Done Jun 8, 2022
@nvoxland nvoxland deleted the remove-shading branch June 8, 2022 19:56
@kataggart kataggart added this to the NEXT milestone Jun 10, 2022
chadlwilson added a commit to chadlwilson/gocd that referenced this pull request Jun 20, 2022
Liquibase 4.12 stopped shading some dependencies, so we can now exclude them properly. It seems our usage does not require these at this stage.

https://github.com/liquibase/liquibase/blob/master/changelog.txt and liquibase/liquibase#2903 are relevant here.
grafjo pushed a commit to urlaubsverwaltung/urlaubsverwaltung that referenced this pull request Nov 25, 2022
grafjo pushed a commit to urlaubsverwaltung/urlaubsverwaltung that referenced this pull request Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants