Merge PSQLDataType with PostgresDataType #213
Conversation
fabianfett
force-pushed
the
ff-merge-psqldatatype-with-postgresdatatype
branch
from
891d693 to
52397d2
Compare
| @@ -16,7 +16,7 @@ public enum PostgresFormatCode: Int16, Codable, CustomStringConvertible { | |||
|
|
|||
| /// The data type's raw object ID. | |||
| /// Use `select * from pg_type where oid = <idhere>;` to lookup more information. | |||
| public struct PostgresDataType: Codable, Equatable, ExpressibleByIntegerLiteral, CustomStringConvertible, RawRepresentable { | |||
There was a problem hiding this comment.
Note: I have moved the Codable and ExpressibleByIntegerLiteral further down and added a remove TODO for the next major version break.
Codecov Report
@@ Coverage Diff @@
## main #213 +/- ##
==========================================
+ Coverage 40.44% 44.83% +4.39%
==========================================
Files 116 116
Lines 7771 7719 -52
==========================================
+ Hits 3143 3461 +318
+ Misses 4628 4258 -370
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| // TODO: The Codable conformance does not make any sense. Let's remove this with next major break. | ||
| extension PostgresDataType: Codable {} | ||
|
|
||
| // TODO: The ExpressibleByIntegerLiteral conformance does not make any sense and is not used anywhere. Remove with next major break. |
There was a problem hiding this comment.
My guess would be that Tanner probably used this during dev to be able to just drop raw data type codes in for quick testing 'cause it felt more convenient than having to add the .init() boilerplate (I can understand the feeling! 😅). Of course, then he either just forgot about it later or looked at it and went "well why not just leave it in" - he was happy to let me take the role of overzealous code neat freak 😆
(I'm 100% in favor of ditching it regardless of all that, all it does is make it easier to misuse the API.)
| @@ -227,3 +222,13 @@ public struct PostgresDataType: Codable, Equatable, ExpressibleByIntegerLiteral, | |||
| return self.knownSQLName ?? "UNKNOWN \(self.rawValue)" | |||
| } | |||
| } | |||
|
|
|||
| // TODO: The Codable conformance does not make any sense. Let's remove this with next major break. | |||
There was a problem hiding this comment.
Was there a point in early Swift 5 before RawRepresentable automatically conferred Codable? That would make sense of why the redundant conformance was left in. (I mean heck, I tend to write out Equatable, Hashable by habit and then grumble at the compiler warning complaining Equatable is redundant...)
Though, given that the conformance is automatically bestowed regardless, does the removal of the explicit declaration actually have to wait for a major version break?
(Hm, do structs get it automatically? Or just enums? Either way, the enum in the other PR could lose it without a major version break at least.)
|
Sigh, merge conflicts. You'd think Git would be smart enough to be able to tell #212 and this are directly related 😒 |
fabianfett
force-pushed
the
ff-merge-psqldatatype-with-postgresdatatype
branch
from
258a149 to
52eb091
Compare
Motivation
While rewriting lot's of PostgresNIO internals last year, I created new types that very closely match existing types. This was a mistake. I should have tried everything to reuse existing types, and change them as needed.
Changes
PSQLDataTypeand replace all uses withPostgresDataTypeResult
Fewer types, easier API for adopters.