-
Notifications
You must be signed in to change notification settings - Fork 432
impl(spanner): remove deprecated ClientOptions #15849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: prepare-for-v3.0.0
Are you sure you want to change the base?
impl(spanner): remove deprecated ClientOptions #15849
Conversation
Summary of ChangesHello @mpeddada1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a significant cleanup effort within the Spanner C++ client library by entirely removing the deprecated Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly removes the deprecated ClientOptions class and its usage throughout the Spanner client library. The changes are mostly cleanups and file removals.
I've left a couple of suggestions:
- There seems to be a leftover constructor in
client.hthat was part of theClientOptionsbackward compatibility and can now be removed. - A test case in
client_test.ccis being removed, but it seems to test an important aspect ofgoogle::cloud::Optionsprecedence. I've suggested restoring it with a more accurate name.
Overall, this is a good cleanup PR.
I am having trouble creating individual review comments. Click here to see my feedback.
google/cloud/spanner/client.h (719-721)
This constructor seems to be a leftover from the removal of the Client(..., ClientOptions const&) constructor. Now that the ClientOptions constructor is gone, there is no ambiguity for Client(conn, {}), so this constructor is no longer needed and can be removed.
google/cloud/spanner/client_test.cc (1316-1334)
This test seems valuable as it verifies that options passed to the Client constructor override options from the Connection. The test name is a bit misleading as it's not testing the deprecated ClientOptions class but google::cloud::Options. I suggest we keep this test and rename it to something more descriptive, like UsesClientConstructorOptions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## prepare-for-v3.0.0 #15849 +/- ##
======================================================
- Coverage 92.95% 92.94% -0.01%
======================================================
Files 2454 2452 -2
Lines 227401 227339 -62
======================================================
- Hits 211380 211304 -76
- Misses 16021 16035 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Removed deprecated client_options files from spanner module. Also removes Client constructor that takes in ClientOptions.