-
Notifications
You must be signed in to change notification settings - Fork 325
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
Fix JDBC non-supported getUpdateCount #1008
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1008 +/- ##
=========================================
Coverage ? 62.62%
Complexity ? 85
=========================================
Files ? 271
Lines ? 11010
Branches ? 1463
=========================================
Hits ? 6895
Misses ? 3679
Partials ? 436
Continue to review full report at Codecov.
|
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'm wondering if we could have something simpler in the advice code. Maybe moving the call to getUpdateCount
alone in the try/catch would make it a bit more readable, and maybe moving this logic to a dedicated helper class.
Also, it's definitely missing one comment that explain why we do this in this case, maybe a unit test would also help to understand here.
@@ -232,6 +262,14 @@ private void testPreparedStatement() throws SQLException { | |||
} | |||
} | |||
|
|||
private void testUpdatePreparedStatement() throws SQLException { | |||
if (updatePreparedStatement != null) { |
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.
this should never happen, we can remove the null
check here.
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.
Looks good to me, only the readme update is missing.
Fixes #1006