Skip to content

Conversation

@thebrianchen
Copy link

Porting from node

@thebrianchen thebrianchen self-assigned this Apr 27, 2021
@thebrianchen thebrianchen requested a review from a team as a code owner April 27, 2021 17:04
@thebrianchen thebrianchen requested a review from a team April 27, 2021 17:04
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/java-firestore API. label Apr 27, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 27, 2021
@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

Merging #611 (114fc26) into master (325452e) will increase coverage by 0.07%.
The diff coverage is 89.74%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #611      +/-   ##
============================================
+ Coverage     73.96%   74.04%   +0.07%     
+ Complexity     1106     1102       -4     
============================================
  Files            67       67              
  Lines          5866     5898      +32     
  Branches        718      721       +3     
============================================
+ Hits           4339     4367      +28     
- Misses         1302     1305       +3     
- Partials        225      226       +1     
Impacted Files Coverage Δ Complexity Δ
...in/java/com/google/cloud/firestore/BulkWriter.java 98.47% <89.74%> (-1.53%) 55.00 <6.00> (+5.00) ⬇️

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 325452e...114fc26. Read the comment docs.

Copy link
Contributor

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

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

yay!

}

return operation.getFuture();
ApiFuture<WriteResult> transformedFuture =
Copy link
Contributor

Choose a reason for hiding this comment

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

A slightly less generic name would do wonders here :)

Copy link
Author

Choose a reason for hiding this comment

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

renamed to processedOperationFuture.

processBufferedOperations();
return ApiFutures.immediateFuture(result);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this not need an executor for the non-deprecated version?

Copy link
Author

Choose a reason for hiding this comment

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

added executor, thanks for catching!

@thebrianchen thebrianchen added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Apr 28, 2021
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Apr 29, 2021
@thebrianchen thebrianchen merged commit a7caff2 into master Apr 29, 2021
@thebrianchen thebrianchen deleted the bc/bulk-buffer branch April 29, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: firestore Issues related to the googleapis/java-firestore API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants