feat(sidekick/rust): decouple option extraction and support resource names (roll-forward)#4385
Conversation
There was a problem hiding this comment.
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.
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
|
looking better, please post a link to a staged full generation run on google-cloud-rust |
069f557 to
8b1c459
Compare
westarle
left a comment
There was a problem hiding this comment.
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.
089f0f1 to
291165f
Compare
Roll forward the resource name generation logic by constructing
ResourceNameTemplateandResourceNameArgsindividually for eachPathBinding, for services withresource_nameoption enabled.Also remove obsolete fields (
ResourceNameTemplateandResourceNameArgs), nest options properly (detailed_tracing_attributesandresource_name), and repair formatted spacing.Fixes #4183