ci(bigtable): update cluster location formatting in integration test#16033
Conversation
product-auto-label
bot
added
the
api: bigtable
Summary of ChangesHello, 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 addresses an issue in the Bigtable instance admin integration tests where cluster location names were not being formatted correctly. The changes ensure that all relevant test cases now properly construct full resource names for zones using the Highlights
Changelog
Activity
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the integration tests to correctly format cluster location names by using Location::FullName(). The changes are correct and address the issue described. I've kept all comments as they suggest marking variables that are not modified after initialization as const, which is a good practice for readability and safety, and aligns with the Google C++ Style Guide. None of the provided rules contradicted or directly covered these comments, so they remain unchanged.
| auto location_a = Location(Project(project_id_), zone_a_); | ||
| auto location_b = Location(Project(project_id_), zone_b_); |
There was a problem hiding this comment.
These variables are not modified after initialization and should be declared const to improve code clarity and prevent accidental modification.
auto const location_a = Location(Project(project_id_), zone_a_);
auto const location_b = Location(Project(project_id_), zone_b_);References
- The Google C++ Style Guide, which this repository follows, recommends using
constfor variables that are not modified after initialization to improve code clarity and prevent accidental modification. (link)
| auto const instance_name = bigtable::InstanceName(project_id_, instance_id); | ||
|
|
||
| auto config = IntegrationTestConfig(instance_id, zone_a_, | ||
| auto location = Location(Project(project_id_), zone_a_); |
There was a problem hiding this comment.
This variable is not modified after initialization and should be declared const to improve code clarity and prevent accidental modification.
auto const location = Location(Project(project_id_), zone_a_);References
- The Google C++ Style Guide, which this repository follows, recommends using
constfor variables that are not modified after initialization to improve code clarity and prevent accidental modification. (link)
|
|
||
| // Create instance | ||
| auto config = IntegrationTestConfig(instance_id, zone_a_); | ||
| auto location = Location(Project(project_id_), zone_a_); |
There was a problem hiding this comment.
This variable is not modified after initialization and should be declared const to improve code clarity and prevent accidental modification.
auto const location = Location(Project(project_id_), zone_a_);References
- The Google C++ Style Guide, which this repository follows, recommends using
constfor variables that are not modified after initialization to improve code clarity and prevent accidental modification. (link)
| auto location_a = Location(Project(project_id_), zone_a_); | ||
| auto location_b = Location(Project(project_id_), zone_b_); |
There was a problem hiding this comment.
These variables are not modified after initialization and should be declared const to improve code clarity and prevent accidental modification.
auto const location_a = Location(Project(project_id_), zone_a_);
auto const location_b = Location(Project(project_id_), zone_b_);References
- The Google C++ Style Guide, which this repository follows, recommends using
constfor variables that are not modified after initialization to improve code clarity and prevent accidental modification. (link)
| auto cluster_config = bigtable::ClusterConfig(location_b.FullName(), 3, | ||
| bigtable::ClusterConfig::HDD); |
There was a problem hiding this comment.
This variable is not modified after initialization and should be declared const to improve code clarity and prevent accidental modification.
auto const cluster_config = bigtable::ClusterConfig(location_b.FullName(), 3,
bigtable::ClusterConfig::HDD);References
- The Google C++ Style Guide, which this repository follows, recommends using
constfor variables that are not modified after initialization to improve code clarity and prevent accidental modification. (link)
| auto const instance_id = RandomInstanceId(generator_); | ||
| auto const instance_name = InstanceName(project_id_, instance_id); | ||
|
|
||
| auto location = Location(Project(project_id_), zone_a_); |
There was a problem hiding this comment.
This variable is not modified after initialization and should be declared const to improve code clarity and prevent accidental modification.
auto const location = Location(Project(project_id_), zone_a_);References
- The Google C++ Style Guide, which this repository follows, recommends using
constfor variables that are not modified after initialization to improve code clarity and prevent accidental modification. (link)
|
|
||
| // Create instance | ||
| auto config = IntegrationTestConfig(instance_id, zone_a_); | ||
| auto location = Location(Project(project_id_), zone_a_); |
There was a problem hiding this comment.
This variable is not modified after initialization and should be declared const to improve code clarity and prevent accidental modification.
auto const location = Location(Project(project_id_), zone_a_);References
- The Google C++ Style Guide, which this repository follows, recommends using
constfor variables that are not modified after initialization to improve code clarity and prevent accidental modification. (link)
|
|
||
| // CompletionQueue `cq` is not being `Run()`, so this should never finish. | ||
| auto const instance_id = RandomInstanceId(generator_); | ||
| auto location = Location(Project(project_id_), zone_a_); |
There was a problem hiding this comment.
This variable is not modified after initialization and should be declared const to improve code clarity and prevent accidental modification.
auto const location = Location(Project(project_id_), zone_a_);References
- The Google C++ Style Guide, which this repository follows, recommends using
constfor variables that are not modified after initialization to improve code clarity and prevent accidental modification. (link)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16033 +/- ##
==========================================
- Coverage 92.64% 92.64% -0.01%
==========================================
Files 2335 2335
Lines 214792 214802 +10
==========================================
+ Hits 198986 198995 +9
- Misses 15806 15807 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Cluster names in the instance admin integration test were not being formatted properly after switching to the generated client.