Skip to content
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 PostgresRowSequence.swift #281

Merged
merged 1 commit into from
Apr 26, 2022
Merged

Update PostgresRowSequence.swift #281

merged 1 commit into from
Apr 26, 2022

Conversation

nicholas-otto
Copy link
Contributor

As mentioned in issue #279 by @fabianfett
As requested in recent issue attached above, the function should be made public. That is the only change made in this pull request and should solve the issue!

As mentioned in issue #279 by fabianfett titled "Make PostgresRowSequence.collect public"
@nicholas-otto nicholas-otto mentioned this pull request Apr 26, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2022

Codecov Report

Merging #281 (c9c0954) into main (def4fe8) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main     #281   +/-   ##
=======================================
  Coverage   44.42%   44.42%           
=======================================
  Files         115      115           
  Lines        9675     9675           
=======================================
  Hits         4298     4298           
  Misses       5377     5377           
Flag Coverage Δ
unittests 44.42% <0.00%> (ø)

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

Impacted Files Coverage Δ
Sources/PostgresNIO/New/PostgresRowSequence.swift 89.73% <0.00%> (ø)

@fabianfett fabianfett self-requested a review April 26, 2022 05:49
Copy link
Collaborator

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

Great. Thanks for tackling this. Since this is a public API change let’s also ask @gwynne for an opinion.

@fabianfett fabianfett requested a review from gwynne April 26, 2022 05:51
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.

Seems fine to me! Unless there was some pressing reason it wasn't public before which still applies at the present time, I say go for it and thank you! 🙂

@gwynne gwynne added the enhancement New feature or request label Apr 26, 2022
@fabianfett fabianfett merged commit 3c0efd7 into vapor:main Apr 26, 2022
@fabianfett fabianfett added this to the 1.10.0 milestone Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants