Skip to content

Conversation

@mpeddada1
Copy link
Collaborator

@mpeddada1 mpeddada1 commented Jul 1, 2025

This PR does the following:


This change is Reviewable

@mpeddada1 mpeddada1 requested a review from a team as a code owner July 1, 2025 20:04
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Jul 1, 2025
@codecov
Copy link

codecov bot commented Jul 1, 2025

Codecov Report

Attention: Patch coverage is 96.46018% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.93%. Comparing base (65a6980) to head (3b7578c).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
google/cloud/spanner/internal/connection_impl.cc 72.72% 3 Missing ⚠️
...gle/cloud/spanner/internal/connection_impl_test.cc 97.91% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #15249    +/-   ##
========================================
  Coverage   92.92%   92.93%            
========================================
  Files        2394     2394            
  Lines      215461   215564   +103     
========================================
+ Hits       200227   200329   +102     
- Misses      15234    15235     +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

Reviewed 9 of 11 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 10 of 11 files reviewed, 1 unresolved discussion


google/cloud/spanner/client.cc line 57 at r2 (raw file):

  auto directed_read_option = ExtractOpt<DirectedReadOption>(opts);
  internal::OptionsSpan span(std::move(opts));
  auto lock_hint = ExtractOpt<LockHintOption>(opts);

Here and in the other places we call ExtractOpt for the LockHintOption, we want to do this before we create the OptionSpan.

@mpeddada1
Copy link
Collaborator Author

Here and in the other places we call ExtractOpt for the LockHintOption, we want to do this before we create the OptionSpan.

Ah good catch, thank you! Done, also added additional testing in client_test.

@mpeddada1 mpeddada1 requested a review from scotthart July 14, 2025 19:25
Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mpeddada1)

@mpeddada1 mpeddada1 merged commit f74959a into googleapis:main Jul 15, 2025
78 of 79 checks passed
@mpeddada1 mpeddada1 deleted the lock_hint_enum branch July 15, 2025 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants