-
-
Notifications
You must be signed in to change notification settings - Fork 336
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
Improvements on spring Jdbi repositories #2544
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some minor comments, generally looks good. LMK if you want to address the comments, otherwise I will merge some time this week (this is a holiday week in the U.S.).
docs/src/adoc/index.adoc
Outdated
@@ -7333,6 +7332,10 @@ To use this module, add a Maven dependency: | |||
</dependency> | |||
---- | |||
|
|||
==== XML-based configuration | |||
For XML-based configurations the class `JdbiFactoryBean` is made available to set up a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we use a link here as well?
docs/src/adoc/index.adoc
Outdated
} | ||
} | ||
---- | ||
<1> This connection factory will enable <<jta, JTA support>>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read this as an anchor to the general JTA documentation, not spring-jta. Could we make this spring-jta
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will, but the difference is small. JTA is an API and Spring JTA is the spring implementation of this API
I will take a look at the review comments |
LMK when you are done; I am planning to do a 3.42.0 release (the Kotlin coroutine stuff and the spring improvements are sufficient for a new release) now that the tests are stable again. |
- reduced use of reflection - documented use - fixed minor bug
5e3bc12
to
1cb5d6f
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
As English in not my first language, please check spelling and grammar.
Made some improvements as suggested by @stevenschlansker
(I fudged my rebase, so I decided to make a new branch instead)