Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

feat: add efficient move support to mutation builder temporaries #989

Merged
merged 2 commits into from
Oct 27, 2019

Conversation

devjgm
Copy link
Contributor

@devjgm devjgm commented Oct 26, 2019

Fixes: #379

Most of the testing for this is done via static_asserts that ensure that a chain of method calls results in a call to .Build() that returns an rvalue. That ensures that the right functions are being called. Then I actually call the code and test that it works, but this part of the test doesn't really verify that moving happened, just that the code path works.


This change is Reviewable

@devjgm devjgm requested a review from coryan October 26, 2019 15:34
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 26, 2019
@codecov
Copy link

codecov bot commented Oct 26, 2019

Codecov Report

Merging #989 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #989      +/-   ##
==========================================
- Coverage   94.26%   94.22%   -0.05%     
==========================================
  Files          95      159      +64     
  Lines        4080    10834    +6754     
==========================================
+ Hits         3846    10208    +6362     
- Misses        234      626     +392
Impacted Files Coverage Δ
google/cloud/spanner/mutations.h 98.21% <100%> (+0.13%) ⬆️
google/cloud/spanner/mutations_test.cc 100% <100%> (ø)
google/cloud/spanner/internal/polling_loop.h 91.89% <0%> (-2.71%) ⬇️
google/cloud/spanner/samples/samples.cc 89.5% <0%> (-0.65%) ⬇️
google/cloud/spanner/transaction.h 100% <0%> (ø) ⬆️
google/cloud/spanner/internal/time_format_test.cc 100% <0%> (ø)
...ogle/cloud/spanner/testing/random_instance_name.cc 100% <0%> (ø)
google/cloud/spanner/retry_policy_test.cc 100% <0%> (ø)
google/cloud/spanner/read_partition_test.cc 100% <0%> (ø)
... and 63 more

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 e6f5d45...9c070e4. Read the comment docs.

Copy link
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

:lgtm: and the testing strategy makes perfect sense.

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@devjgm devjgm merged commit 4f01799 into googleapis:master Oct 27, 2019
@devjgm devjgm deleted the moving-builders branch October 27, 2019 01:32
devjgm added a commit to devjgm/google-cloud-cpp that referenced this pull request May 7, 2020
…gleapis/google-cloud-cpp-spanner#989)

Fixes: googleapis/google-cloud-cpp-spanner#379

Most of the testing for this is done via static_asserts that ensure that a chain of method calls results in a call to .Build() that returns an rvalue. That ensures that the right functions are being called. Then I actually call the code and test that it works, but this part of the test doesn't really verify that moving happened, just that the code path works.
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.

Add && overloads from internal::WriteMutationBuilder::{Add,Emplace}Row.
3 participants