-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Update package for Sendable and 100% test coverage #223
Conversation
…ncoder/decoders to the SQLKit layer. Add support for new SQLKit functionality. Throw DecodingErrors instead of fatalErrors(). Remove unused properties and files.
…d missing FluentBenchmark test set. Use singleton ELG and thread pool for tests.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #223 +/- ##
===========================================
+ Coverage 77.86% 98.87% +21.00%
===========================================
Files 7 7
Lines 253 266 +13
===========================================
+ Hits 197 263 +66
+ Misses 56 3 -53
|
|
||
|
||
/// This is here because it allows for full test coverage; it serves no actual purpose functionally. | ||
/*private*/ func ignoreRow(_: MySQLRow) throws {} |
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.
Do we want to remove the access modifier?
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.
Needed in order to use it in tests. Also, "remove"? This code is new in this PR.
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 meant just remove the commented out private
} | ||
} | ||
|
||
private struct LastInsertRow: DatabaseOutput { | ||
/// A `DatabaseOutput` used to provide last insert IDs from query metadata to the Fluent layer. | ||
/*private*/ struct LastInsertRow: DatabaseOutput { |
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.
Access modifier again
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.
Again, needed in tests.
} | ||
} | ||
|
||
/// Retroactive conformance to `Sendable` for `MySQLConnection`, which happens to actually be `Senable`-correct | ||
/// but not annotated as such. | ||
extension MySQLNIO.MySQLConnection: @unchecked Swift.Sendable {} |
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.
Would it be better to do this with @preconcurrency import MySQLNIO
- this would provide a warning when it's no longer needed and avoid the situation where it might accidentally be not-Sendable-correct in the future
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.
MySQLNIO is most of the way done being completely rewritten - MySQLConnection
will no longer be the type in use; it isn't expected to be changed again at all.
These changes are now available in 4.5.0
Adds
Sendable
correctness andExistentialAny
compliance, bumps min Swift version to 5.8, leverages the new SQLKit functionality, throws errors instead of crashes, updates CI, modernizes README, and achieves 100% test and documentation coverage.