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

feat: add caller push of binary data (rebase of #953) #1659

Merged
merged 9 commits into from Dec 26, 2019

Conversation

@paplorinc
Copy link
Contributor

paplorinc commented Dec 23, 2019

All Submissions:

Rebase and fix of #953.

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Does mvn checkstyle:check pass ?
  3. Have you added your new test classes to an existing test suite in alphabetical order?

Changes to Existing Features:

  • Does this break existing behaviour? If so please explain.
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?
tomdcc and others added 8 commits Sep 21, 2017
Allows a caller to push binary data to an output stream rather than
having to provide a byte array or input stream. This gives the caller
more control over buffering strategy and allows explicit cleanup of
off-heap buffers or other non-garbage-collected resources.
@paplorinc

This comment has been minimized.

Copy link
Contributor Author

paplorinc commented Dec 23, 2019

@davecramer, addressed your concerns from #1656 (review) (pushed it again properly).
cc: @tomdcc

@paplorinc paplorinc changed the title feat: add caller push of binary data (rebase or #953) feat: add caller push of binary data (rebase of #953) Dec 23, 2019
@davecramer

This comment has been minimized.

Copy link
Member

davecramer commented Dec 23, 2019

Thanks!
[WARNING] /home/travis/build/pgjdbc/pgjdbc/pgjdbc/target/gen-src/org/postgresql/util/ByteStreamWriter.java:62: warning: no @return
is one of the travis errors

@paplorinc

This comment has been minimized.

Copy link
Contributor Author

paplorinc commented Dec 23, 2019

@davecramer, thanks for your quick review - one test (testFastCloses) seems to fail for a possibly unrelated reason - is it known flaky or should I investigate?

@davecramer

This comment has been minimized.

Copy link
Member

davecramer commented Dec 23, 2019

ya, it's flaky, no need to muck with that one, unless you feel so inclined but I'd prefer if that was fixed in another PR

@paplorinc

This comment has been minimized.

Copy link
Contributor Author

paplorinc commented Dec 23, 2019

Is there anything else you would like us to do here?

@davecramer

This comment has been minimized.

Copy link
Member

davecramer commented Dec 24, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 24, 2019

Codecov Report

Merging #1659 into master will increase coverage by 0.09%.
The diff coverage is 87.71%.

@@             Coverage Diff              @@
##             master    #1659      +/-   ##
============================================
+ Coverage     68.79%   68.89%   +0.09%     
- Complexity     4080     4104      +24     
============================================
  Files           181      183       +2     
  Lines         16957    17014      +57     
  Branches       2789     2794       +5     
============================================
+ Hits          11666    11721      +55     
- Misses         4011     4015       +4     
+ Partials       1280     1278       -2
@paplorinc

This comment has been minimized.

Copy link
Contributor Author

paplorinc commented Dec 24, 2019

Thanks @davecramer and @tomdcc.
Happy Christmas!

@davecramer davecramer merged commit db228a4 into pgjdbc:master Dec 26, 2019
3 checks passed
3 checks passed
codecov/project 68.89% (+0.09%) compared to a44ab4b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@paplorinc paplorinc deleted the paplorinc:byte-stream-writer branch Dec 27, 2019
@tomdcc

This comment has been minimized.

Copy link
Contributor

tomdcc commented Jan 6, 2020

Thanks @davecramer and thanks very much @paplorinc for brushing this back up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.