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

Improvements on spring Jdbi repositories #2544

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

xfredk
Copy link
Contributor

@xfredk xfredk commented Nov 20, 2023

  • reduced use of reflection
  • documented usage
  • fixed minor bug

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)

Copy link
Contributor

@hgschmie hgschmie left a 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.).

@@ -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
Copy link
Contributor

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?

}
}
----
<1> This connection factory will enable <<jta, JTA support>>.
Copy link
Contributor

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 ?

Copy link
Contributor Author

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

docs/src/adoc/index.adoc Show resolved Hide resolved
@xfredk
Copy link
Contributor Author

xfredk commented Nov 22, 2023

I will take a look at the review comments

@hgschmie
Copy link
Contributor

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.

xfredk and others added 2 commits November 27, 2023 18:18
- reduced use of reflection
- documented use
- fixed minor bug
@hgschmie hgschmie force-pushed the jdbi-repositories_improvements branch from 5e3bc12 to 1cb5d6f Compare November 28, 2023 02:19
Copy link

sonarcloud bot commented Nov 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@hgschmie hgschmie merged commit 76b1f48 into jdbi:master Nov 28, 2023
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants