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

Scope fixes for the Spring extension #53

Merged
merged 7 commits into from
Mar 2, 2021
Merged

Conversation

tsg21
Copy link
Contributor

@tsg21 tsg21 commented Jun 20, 2020

Fixes #44

@@ -59,18 +62,22 @@
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<scope>test</scope>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Were these dependencies deliberately not "test"?

Copy link
Member

Choose a reason for hiding this comment

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

They're set to test in the parent dependencyManagement.

https://github.com/gruelbox/transaction-outbox/blob/master/pom.xml#L129

@tsg21 tsg21 marked this pull request as ready for review June 20, 2020 15:47
Copy link
Member

@badgerwithagun badgerwithagun left a comment

Choose a reason for hiding this comment

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

By making the Spring dependencies provided, we're setting up for a situation where a breaking change in Spring causes odd runtime failures with transaction-outbox later. I wonder if this is one of those rare cases where it's worth sticking with the default scope and investigating a version range option for the published POM? That way we can safely and gracefully up or downgrade only to tested versions.

Pretty sure the test scope additions are unnecessary.

@badgerwithagun
Copy link
Member

@tsg21, It'd be really useful if you could spare a moment to give 2.1.13-SNAPSHOT in the Sonatype snapshots repository (OSSRH) a try. It's a build of #40 which, although primarily a refactor to a non-blocking core, makes a number of (what I think are) improvements to the Spring side of things which would affect you, and I lack real-world apps to test against.

@badgerwithagun
Copy link
Member

bump @tsg21

@badgerwithagun badgerwithagun merged commit f16f1b2 into gruelbox:master Mar 2, 2021
badgerwithagun pushed a commit that referenced this pull request Aug 21, 2021
* Remove dependency on Hibernate and use Spring's utilities to integrate with spring transactions.

* Move the check for an active transaction. Remove an unnecessary try-catch.

* Switch spring dependencies to "provided" scope and test dependencies to "test" scope.

* Remove duplicate jdbc dependency.

* Remove duplicated test scope directives

Co-authored-by: Graham Crockford <6483013+badgerwithagun@users.noreply.github.com>
(cherry picked from commit f16f1b2)
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.

transactionoutbox-spring depends on specific Spring version
2 participants