Skip to content

Remove need to supply PostgresDecodingContext on decode #307

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

Conversation

fabianfett
Copy link
Collaborator

Motivation

To use a customized JSONDecoder, users have the option to supply a PostgresDecodingContext. Currently we require users to set explicitly on every row decode call. This makes the API unnecessarily verbose. Since the PostgresDecodingContext is generic we can not default the parameter. Instead we need add proxy functions so that we can default the parameter manually.

Changes

  • Add decode methods omitting the PostgresDecodingContext

@fabianfett fabianfett requested a review from gwynne October 3, 2022 12:55
@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2022

Codecov Report

Merging #307 (17ba80f) into main (17ba80f) will not change coverage.
The diff coverage is n/a.

❗ Current head 17ba80f differs from pull request most recent head a2affaf. Consider uploading reports for the commit a2affaf to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #307   +/-   ##
=======================================
  Coverage   43.94%   43.94%           
=======================================
  Files         115      115           
  Lines        9670     9670           
=======================================
  Hits         4249     4249           
  Misses       5421     5421           
Flag Coverage Δ
unittests 43.94% <0.00%> (ø)

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

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.

LGTM

@fabianfett
Copy link
Collaborator Author

The API breakage detector reports a false positive here. The decode methods have been moved from PostgresRowSequence to extension AsyncSequence where Element == PostgresRow, which PostgresRowSequence conforms to.

@fabianfett fabianfett merged commit 2cad52a into vapor:main Oct 3, 2022
@fabianfett fabianfett deleted the ff-remove-need-to-supply-context-on-decode branch October 3, 2022 14:14
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