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

fixes #543 change hibernate-validator and expressly to test scope #544

Closed
wants to merge 1 commit into from

Conversation

stevehu
Copy link

@stevehu stevehu commented Mar 4, 2025

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:

Special notes for reviewers:

Additional information (if needed):

… to test scope
@stevehu stevehu requested a review from fjtirado as a code owner March 4, 2025 14:53
<artifactId>hibernate-validator</artifactId>
<groupId>org.hibernate.validator</groupId>
<artifactId>hibernate-validator</artifactId>
<scope>test</scope>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually these are neeeed at runtime, not just for test, so it should be runtime not test

<artifactId>expressly</artifactId>
<groupId>org.glassfish.expressly</groupId>
<artifactId>expressly</artifactId>
<scope>test</scope>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

@@ -34,6 +34,16 @@
<groupId>net.thisptr</groupId>
<artifactId>jackson-jq</artifactId>
</dependency>
<dependency>
Copy link
Collaborator

@fjtirado fjtirado Mar 4, 2025

Choose a reason for hiding this comment

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

Once you made the other runtime, this does not need to be added and this file should remain unchanged

@@ -21,6 +21,16 @@
<groupId>io.serverlessworkflow</groupId>
<artifactId>serverlessworkflow-impl-core</artifactId>
</dependency>
<dependency>
Copy link
Collaborator

@fjtirado fjtirado Mar 4, 2025

Choose a reason for hiding this comment

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

Same, not needed once you set the dependency to runtime, this file should not be modified

Copy link
Collaborator

@fjtirado fjtirado left a comment

Choose a reason for hiding this comment

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

These two dependencies are needed for the SDK to work on runtime, so they should be changed to runtime not to test
That way, you do not need to change the impl poms

<artifactId>hibernate-validator</artifactId>
<groupId>org.hibernate.validator</groupId>
<artifactId>hibernate-validator</artifactId>
<scope>test</scope>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<scope>test</scope>
<scope>runtime</scope>

<artifactId>expressly</artifactId>
<groupId>org.glassfish.expressly</groupId>
<artifactId>expressly</artifactId>
<scope>test</scope>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<scope>test</scope>
<scope>runtime</scope>

@stevehu stevehu closed this Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants