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

Make OperationFutureImpl support ApiFutures.catching() #527

Merged
merged 2 commits into from
May 2, 2018

Conversation

yihanzhen
Copy link
Contributor

No description provided.

@yihanzhen
Copy link
Contributor Author

@codecov-io
Copy link

codecov-io commented Apr 25, 2018

Codecov Report

Merging #527 into master will decrease coverage by 0.19%.
The diff coverage is 9.09%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #527     +/-   ##
===========================================
- Coverage     72.36%   72.17%   -0.2%     
+ Complexity      864      863      -1     
===========================================
  Files           171      171             
  Lines          3952     3960      +8     
  Branches        306      306             
===========================================
- Hits           2860     2858      -2     
- Misses          948      958     +10     
  Partials        144      144
Impacted Files Coverage Δ Complexity Δ
...m/google/api/gax/longrunning/OperationFutures.java 54.16% <0%> (-2.36%) 1 <0> (ø)
...oogle/api/gax/longrunning/OperationFutureImpl.java 68.29% <10%> (-19.95%) 11 <1> (-1)

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 a9aef28...d70b713. Read the comment docs.

ApiFunction<OperationSnapshot, ResponseT> responseTransformer,
ApiFunction<OperationSnapshot, MetadataT> metadataTransformer,
ApiFunction<Exception, ResponseT> exceptionTransformer) {
this.pollingFuture = checkNotNull(pollingFuture);

This comment was marked as spam.

This comment was marked as spam.

@yihanzhen
Copy link
Contributor Author

This PR comes with the PR in google-cloud-java.

@yihanzhen
Copy link
Contributor Author

Another look please @garrettjonesgoogle

@garrettjonesgoogle
Copy link
Member

I don't have a concern with this - I will defer to @pongad for an LGTM.

@pongad
Copy link
Contributor

pongad commented May 1, 2018

In that case, can we get rid of pollingFuture.get() calls? I don't think they are needed anyway. That'd make me a lot more comfortable with this.

@yihanzhen
Copy link
Contributor Author

@pongad PTAL

@yihanzhen yihanzhen merged commit d8c0686 into googleapis:master May 2, 2018
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.

4 participants