Skip to content

Cleanup PostgresDecodable #241

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

Merged
merged 1 commit into from
Mar 5, 2022
Merged

Conversation

fabianfett
Copy link
Collaborator

Motivation

Before we make the PostgresDecodable public, let's revisit it. In Swift prefer initializer over static named methods for type creation.

Changes

  • Replace the static func decode with an init
  • Cleanup the default implementations. Put the impl. into their correct protocol conformance.

@@ -35,12 +35,12 @@ protocol PostgresDecodable {
/// - context: A `PSQLDecodingContext` providing context for decoding. This includes a `JSONDecoder`
/// to use when decoding json and metadata to create better errors.
/// - Returns: A decoded object
static func decode<JSONDecoder: PostgresJSONDecoder>(
init<JSONDecoder: PostgresJSONDecoder>(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gwynne This is the real meat of this PR. Everything else is cleanup.

@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2022

Codecov Report

Merging #241 (8d9f229) into main (d16467d) will increase coverage by 4.04%.
The diff coverage is 77.96%.

@@            Coverage Diff             @@
##             main     #241      +/-   ##
==========================================
+ Coverage   39.56%   43.61%   +4.04%     
==========================================
  Files         115      115              
  Lines        9478     9486       +8     
==========================================
+ Hits         3750     4137     +387     
+ Misses       5728     5349     -379     
Flag Coverage Δ
unittests 43.61% <77.96%> (+4.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ces/PostgresNIO/New/Data/Int+PostgresCodable.swift 30.71% <37.14%> (+5.22%) ⬆️
Sources/PostgresNIO/New/PostgresCodable.swift 91.66% <66.66%> (ø)
...s/PostgresNIO/New/Data/Float+PostgresCodable.swift 76.66% <85.71%> (+3.33%) ⬆️
...PostgresNIO/New/Data/Decimal+PostgresCodable.swift 74.19% <91.66%> (+6.45%) ⬆️
...s/PostgresNIO/New/Data/Array+PostgresCodable.swift 94.73% <100.00%> (+1.11%) ⬆️
...es/PostgresNIO/New/Data/Bool+PostgresCodable.swift 95.34% <100.00%> (+4.65%) ⬆️
...s/PostgresNIO/New/Data/Bytes+PostgresCodable.swift 73.52% <100.00%> (ø)
...es/PostgresNIO/New/Data/Date+PostgresCodable.swift 89.28% <100.00%> (ø)
...es/PostgresNIO/New/Data/JSON+PostgresCodable.swift 100.00% <100.00%> (ø)
...IO/New/Data/RawRepresentable+PostgresCodable.swift 82.35% <100.00%> (ø)
... and 32 more

@fabianfett fabianfett requested a review from gwynne March 3, 2022 08:56
Copy link
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fabianfett fabianfett merged commit ef425af into vapor:main Mar 5, 2022
@fabianfett fabianfett deleted the ff-decode-init branch March 5, 2022 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants