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

Release liquibase-cdi-jakarta to maven repositories #4001

Merged

Conversation

DCCSKrezovic
Copy link

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This updates the github actions to include the newly added liquibase-cdi-jakarta module.

Fixes #3962

@nvoxland
Copy link
Contributor

Initial/Pre-Review Thoughts

Thanks for the updates to the release logic.

Questions I have:

  • Have we looked at the pom.xml that gets distributed in the liquibase-cdi-jakarta.jar to make sure it's what users need/want and not build-related stuff in there?
  • Are we happy with the maven coordinates before the first release?
    • liquibase-cdi-jakarta seems better than liquibase-jakarta-cdi? (I think so).
    • Is it better or worse to use "jakarta" in the version for the same liquibase-cdi name? (I'm not sure off hand)

Potential risks:

  • Can't really test it until release time

What could make the full review difficult:

  • Nothing

@DCCSKrezovic
Copy link
Author

Hi @nvoxland, thanks for the review.

  • It's exactly the same pom.xml as for liquibase-cdi (non-jakarta), except the Jakarta-specific dependencies, obviously. So, the final dependency here is just the cdi-api (provided) and liquibase-core (required)
  • The naming has been agreed and discussed in Jakartaee CDI for liquibase #3642

@kevin-atx kevin-atx added TypeEnhancement RiskLow Trivial changes in spelling, documentation changes, focused bug fixes, etc. labels Mar 28, 2023
@filipelautert filipelautert self-assigned this Mar 31, 2023
@filipelautert filipelautert added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Mar 31, 2023
Copy link
Contributor

@filipelautert filipelautert left a comment

Choose a reason for hiding this comment

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

In 4.20.0 we added the new module. Now we need to release it, thanks for the PR @DCCSKrezovic !

@XDelphiGrl
Copy link
Contributor

hi, @filipelautert! The liquibase-artifacts.zip from this PR's build do not contain the renamed CDI. Is that expected?

Contents of liquibase-artifacts.zip
image


cp liquibase-cdi-jakarta/target/liquibase-cdi-jakarta-0-SNAPSHOT.jar artifacts
cp liquibase-cdi-jakarta/target/liquibase-cdi-jakarta-0-SNAPSHOT-sources.jar artifacts
cp liquibase-cdi-jakarta/target/liquibase-cdi-jakarta-0-SNAPSHOT-javadoc.jar artifacts
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @XDelphiGrl - the copy commands are here , but as we execute the scripts from master they will only run when this pr is merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the explanation, @filipelautert. I'll go ahead and do my code review and we can get this merged. I'll check that the build produces the correctly named artifacts.

Copy link
Contributor

@XDelphiGrl XDelphiGrl left a comment

Choose a reason for hiding this comment

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

Coming soon to a Maven near you ... liquibase-cdi-jakarta! This PR updates build logic to for the CDI artifacts. The sign-artifacts.sh script generates the MD5 and SHA1at the directory level; the renamed CDI artifacts will go through the same signing process as the previous CDI artifacts.

  • The functionality of these build changes cannot be validated until they are merged to master.
  • QA will check the generated artifacts from the concomitant master build for correctness.
  • Test harness and functional tests do not exercise CDI; results unnecessary.

APPROVED

@filipelautert filipelautert merged commit cd6fcea into liquibase:master Apr 5, 2023
43 of 47 checks passed
@filipelautert filipelautert added this to the 1NEXT milestone Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RiskLow Trivial changes in spelling, documentation changes, focused bug fixes, etc. SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions TypeEnhancement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Publish module liquibase-cdi-jakarta to Maven Central repository
6 participants