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

feat!: replaces PartitionOptions proto with struct #1035

Merged
merged 7 commits into from
Nov 11, 2019

Conversation

devjgm
Copy link
Contributor

@devjgm devjgm commented Nov 11, 2019

Fixes #452

Now that PartitionOptions is a struct, it should likely be passed by const-ref and not moved. Well, clang-tidy yells at me otherwise.

This is #1030 take two, because I accidentally closed that PR instead of merging it. Why didn't I just reopen that PR? Because I pushed the branch again after and now it won't let me reopen that PR.


This change is Reviewable

@devjgm devjgm requested a review from mr-salty November 11, 2019 16:48
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 11, 2019
@codecov
Copy link

codecov bot commented Nov 11, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1035      +/-   ##
==========================================
+ Coverage   93.66%   93.68%   +0.01%     
==========================================
  Files         158      161       +3     
  Lines       11215    11241      +26     
==========================================
+ Hits        10505    10531      +26     
  Misses        710      710
Impacted Files Coverage Δ
google/cloud/spanner/read_options.h 100% <ø> (ø) ⬆️
google/cloud/spanner/client.h 100% <ø> (ø) ⬆️
google/cloud/spanner/connection.h 100% <ø> (ø) ⬆️
google/cloud/spanner/partition_options.cc 100% <100%> (ø)
google/cloud/spanner/partition_options.h 100% <100%> (ø)
google/cloud/spanner/client.cc 82.82% <100%> (ø) ⬆️
google/cloud/spanner/internal/connection_impl.cc 96.08% <100%> (ø) ⬆️
google/cloud/spanner/partition_options_test.cc 100% <100%> (ø)
google/cloud/spanner/keys.h 100% <0%> (ø) ⬆️
... and 2 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 10950f5...7a2cdc0. Read the comment docs.

@mr-salty
Copy link
Contributor

taking it mostly on faith this is the same as the other PR and not a trojan :)

Copy link
Contributor

@mr-salty mr-salty left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 12 files reviewed, all discussions resolved (waiting on @mr-salty)

@devjgm devjgm merged commit 3157ba4 into googleapis:master Nov 11, 2019
@devjgm devjgm deleted the partition-options branch November 11, 2019 17:33
devjgm added a commit to devjgm/google-cloud-cpp that referenced this pull request May 7, 2020
…-cloud-cpp-spanner#1035)

Fixes googleapis/google-cloud-cpp-spanner#452

Now that PartitionOptions is a struct, it should likely be passed by const-ref and not moved. Well, clang-tidy yells at me otherwise.

This is googleapis/google-cloud-cpp-spanner#1030 take two, because I accidentally closed that PR instead of merging it. Why didn't I just reopen that PR? Because I pushed the branch again after and now it won't let me reopen that PR.
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.

Consider: writing a wrapper for google::spanner::v1::PartitionOptions.
3 participants