-
Notifications
You must be signed in to change notification settings - Fork 30
feat(sidekick): Generate samples for single value setters #2263
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
Conversation
4de6483
to
7a2fdfa
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2263 +/- ##
==========================================
- Coverage 84.15% 84.15% -0.01%
==========================================
Files 97 97
Lines 9794 9875 +81
==========================================
+ Hits 8242 8310 +68
- Misses 1206 1214 +8
- Partials 346 351 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This is now ready for review, with googleapis/google-cloud-rust#3369 as a counterpart. |
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.
A few nits and test improvements.
internal/sidekick/internal/rust/templates/common/setter_preamble/singular_option.mustache
Outdated
Show resolved
Hide resolved
internal/sidekick/internal/rust/templates/common/setter_preamble/singular_value.mustache
Outdated
Show resolved
Hide resolved
7a2fdfa
to
99e4e8c
Compare
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.
All change requests addressed in the last commit.
internal/sidekick/internal/rust/templates/common/setter_preamble/singular_option.mustache
Outdated
Show resolved
Hide resolved
internal/sidekick/internal/rust/templates/common/setter_preamble/singular_value.mustache
Outdated
Show resolved
Hide resolved
// IsEnum returns true if the primitive type of a field is `ENUM_TYPE`. | ||
// | ||
// This is useful for mustache templates that differ only | ||
// in the broad category of field type involved. | ||
func (f *Field) IsEnum() bool { | ||
return f.Typez == ENUM_TYPE | ||
} |
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.
I think this change uncovered a number of bugs... Some of our mustache templates assumed this attribute was present, e.g.:
librarian/internal/sidekick/internal/rust/templates/convert-prost/oneof.mustache
Lines 38 to 43 in 014b5f4
{{^IsEnum}} | |
Self::{{Codec.BranchName}}(v) => Ok(T::from_{{Codec.SetterName}}(v.cnv()?)), | |
{{/IsEnum}} | |
{{#IsEnum}} | |
Self::{{Codec.BranchName}}(v) => Ok(T::from_{{Codec.SetterName}}(v)), | |
{{/IsEnum}} |
But the attribute did not exist. Sigh... this is a thing I hate about the implementation of mustache templates in go.
Librarian Version: v0.0.0-20250925031739-cb21cf1ae8af Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/librarian-release-container:latest <details><summary>librarian: 0.3.0</summary> ## [0.3.0](v0.2.0...v0.3.0) (2025-09-25) ### Features * discovery-based APIs and pagination (#2350) ([cb21cf1](cb21cf1)) * Make generated `ProtoMessage` and `ProtoEnum` classes `final` (#2349) ([7d0520b](7d0520b)) * Require that all imports have a version contraints (#2331) Fixes #1989 This should not be landed before googleapis/google-cloud-rust#3396 ([00828d5](00828d5)) * Generate samples for single value setters (#2263) ([f7c0b84](f7c0b84)) * discovery doc arrays (#2337) ([da69195](da69195)) * Inject InstrumentationClientInfo for tracing (#2252) - Add static INSTRUMENTATION_CLIENT_INFO to lib.rs.mustache. - Use INSTRUMENTATION_CLIENT_INFO in transport.rs.mustache if tracing is enabled. For #2212 see also googleapis/google-cloud-rust#3347 and googleapis/google-cloud-rust#3376 ([1358226](1358226)) * parse most object fields (#2318) Parse most fields of object in a discovery doc. Fields with an inline type definition still need some custom work. ([f2d1a10](f2d1a10)) * Add the ability to insert text after the package title (#2323) The current use case for this is to advise users of equivalent Firebase packages. For example: ```toml readme-after-title-text = """> [!TIP] > Flutter applications should use [Firebase AI Logic](https://firebase.google.com/products/firebase-ai-logic). > > The Generate Language API is meant for Dart desktop and cloud applications. > Firebase AI Logic provides client-side access to both the Gemini Developer > API and Vertex AI. """ ``` Which results in a README.md that looks like: <img width="1485" height="909" alt="image" src="https://github.com/user-attachments/assets/a1c9120e-eafd-4394-9562-48c595ab4960" /> ([756e72f](756e72f)) ### Bug Fixes * read version from version.txt file (#2347) Fixes #2348 Moves version.txt to the `internal/cli` package so it can be read by the `embed` package as a variable. When constructing the synthetic version number, use this release version as the base. ([014b5f4](014b5f4)) * race condition in createWorkRoot() (#2338) Creating a temporary directory based on a timestamp is inherently racy. Use the standard functions to create temporary directories, and relax the tests to check for what matters. ([46428ca](46428ca)) * parse github remote from local directory (#2328) Fixes #2327 ([1c71bd9](1c71bd9)) </details>
No description provided.