Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

fix default channelPoolSize to be 1 (instead of 0) #443

Merged
merged 1 commit into from
Dec 13, 2017

Conversation

igorbernstein2
Copy link
Contributor

Fix a bug introduced in #436

@codecov-io
Copy link

codecov-io commented Dec 13, 2017

Codecov Report

Merging #443 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #443      +/-   ##
============================================
+ Coverage      69.1%   69.11%   +0.01%     
  Complexity      635      635              
============================================
  Files           140      140              
  Lines          3013     3014       +1     
  Branches        230      230              
============================================
+ Hits           2082     2083       +1     
  Misses          843      843              
  Partials         88       88
Impacted Files Coverage Δ Complexity Δ
...api/gax/grpc/InstantiatingGrpcChannelProvider.java 40.17% <100%> (+0.51%) 8 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2646f08...1f09e37. Read the comment docs.

@vam-google vam-google self-requested a review December 13, 2017 20:15
Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

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

LGTM.
Please also add validation in InstantiatingGrpcChannelProvider() constructor before merging.

@igorbernstein2
Copy link
Contributor Author

Validation already exists on the setter:
https://github.com/googleapis/gax-java/pull/443/files#diff-25ba434a47d99bcc0b998965a298661cR352

I don't have permissions to merge PRs. Please merge on my behalf

@vam-google
Copy link
Contributor

Yes, the validation exists in the setter of the builder, but ideally class itself should be responsible for not allowing invalid initialization values, not the builder. It is a minor thing, so whatever, merging as is.

@vam-google vam-google merged commit 3a13293 into googleapis:master Dec 13, 2017
@igorbernstein2 igorbernstein2 deleted the fix-channel-pool branch January 10, 2018 05:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants