Skip to content

feat(sidekick/rust): decouple option extraction and support resource names (roll-forward)#4385

Merged
haphungw merged 4 commits intogoogleapis:mainfrom
haphungw:feat-rust-resource-names-rollforward
Mar 10, 2026
Merged

feat(sidekick/rust): decouple option extraction and support resource names (roll-forward)#4385
haphungw merged 4 commits intogoogleapis:mainfrom
haphungw:feat-rust-resource-names-rollforward

Conversation

@haphungw
Copy link
Copy Markdown
Contributor

@haphungw haphungw commented Mar 7, 2026

Roll forward the resource name generation logic by constructing ResourceNameTemplate and ResourceNameArgs individually for each PathBinding, for services with resource_name option enabled.

Also remove obsolete fields (ResourceNameTemplate and ResourceNameArgs), nest options properly (detailed_tracing_attributes and resource_name), and repair formatted spacing.

Fixes #4183

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 refactors the resource name generation logic for the Rust sidekick, decoupling it to operate on a per-PathBinding basis. This is a good architectural improvement that makes the logic more modular and easier to follow. The changes are consistently applied across the Go annotation code, the tests, and the Rust templates. The tests have been updated effectively to cover the new per-binding logic. I have one suggestion to improve maintainability by removing some fields from the method-level annotation that appear to have become obsolete with this refactoring.

Note: Security Review did not run due to the size of the PR.

Comment thread internal/sidekick/rust/annotate.go Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.67%. Comparing base (240a9d6) to head (291165f).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4385      +/-   ##
==========================================
- Coverage   81.24%   80.67%   -0.58%     
==========================================
  Files         105      106       +1     
  Lines        8580     8723     +143     
==========================================
+ Hits         6971     7037      +66     
- Misses       1125     1193      +68     
- Partials      484      493       +9     

☔ 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.

@haphungw haphungw marked this pull request as ready for review March 7, 2026 02:09
@haphungw haphungw requested a review from a team as a code owner March 7, 2026 02:09
@westarle
Copy link
Copy Markdown
Contributor

westarle commented Mar 7, 2026

looking better, please post a link to a staged full generation run on google-cloud-rust

Comment thread internal/sidekick/rust/templates/crate/src/transport.rs.mustache
Comment thread internal/sidekick/rust/templates/crate/src/transport.rs.mustache Outdated
@haphungw haphungw force-pushed the feat-rust-resource-names-rollforward branch from 069f557 to 8b1c459 Compare March 9, 2026 21:07
@haphungw haphungw requested a review from westarle March 9, 2026 21:40
Comment thread internal/sidekick/rust/templates/crate/src/transport.rs.mustache Outdated
Copy link
Copy Markdown
Contributor

@westarle westarle left a comment

Choose a reason for hiding this comment

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

The results in googleapis/google-cloud-rust#4936 look good for DNS. If you only enable DNS, and remove the extra blank lines, I think you can submit this and then work on the small bug I noticed in that PR.

Comment thread internal/sidekick/rust/templates/crate/src/transport.rs.mustache Outdated
@haphungw haphungw force-pushed the feat-rust-resource-names-rollforward branch from 089f0f1 to 291165f Compare March 10, 2026 16:38
@haphungw haphungw merged commit b5780b7 into googleapis:main Mar 10, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sidekick/rust: implement helper to generate resource name

3 participants