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

Conversation

@andreamlin
Copy link
Contributor

Make class HttpRequestRunnable an AutoValue class for simplicity.

@andreamlin
Copy link
Contributor Author

PTAL

@codecov-io
Copy link

codecov-io commented Mar 23, 2018

Codecov Report

Merging #509 into master will decrease coverage by 0.11%.
The diff coverage is 60%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #509      +/-   ##
============================================
- Coverage        75%   74.88%   -0.12%     
  Complexity      971      971              
============================================
  Files           183      183              
  Lines          4236     4216      -20     
  Branches        335      335              
============================================
- Hits           3177     3157      -20     
  Misses          906      906              
  Partials        153      153
Impacted Files Coverage Δ Complexity Δ
...oogle/api/gax/httpjson/ManagedHttpJsonChannel.java 2% <0%> (ø) 1 <0> (ø) ⬇️
...m/google/api/gax/httpjson/HttpRequestRunnable.java 54.16% <62.5%> (-13.49%) 5 <2> (ø)

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 1dd8124...4ec6e41. Read the comment docs.

@andreamlin andreamlin force-pushed the httprequestrunnable_autovalue branch from 924d3eb to e83bc07 Compare March 26, 2018 21:09
@andreamlin andreamlin force-pushed the httprequestrunnable_autovalue branch from e83bc07 to e4377de Compare March 26, 2018 21:11
@garrettjonesgoogle garrettjonesgoogle requested review from vam-google and removed request for michaelbausor October 19, 2018 18:07
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, with one comment (please address that or provide additional details explaining why the current implementation is correct).

.setApiMethodDescriptor(methodDescriptor)
.setHttpTransport(new MockHttpTransport())
.setJsonFactory(new JacksonFactory())
.setResponseFuture(SettableApiFuture.<Void>create())

This comment was marked as spam.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The response future should not be nullable.

It seems common in google-cloud-java to have a Void type that represents the response type.

@sduskis
Copy link
Contributor

sduskis commented Jan 22, 2019

@andreamlin, is this PR ready to be merged?

@andreamlin
Copy link
Contributor Author

@sduskis ack
I will make the changes requested by the reviewer soon and then merge.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 22, 2019
@andreamlin andreamlin merged commit cb78601 into googleapis:master Jan 23, 2019
@andreamlin andreamlin deleted the httprequestrunnable_autovalue branch January 23, 2019 19:31
This was referenced Jan 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants