Skip to content

refactor(internal/sidekick/rust): generate convert as top level module - #7531

Merged
suzmue merged 2 commits into
googleapis:mainfrom
suzmue:convert-module-cleanup
Sep 10, 2026
Merged

suzmue merged 2 commits into
googleapis:mainfrom
suzmue:convert-module-cleanup

Conversation

@suzmue

@suzmue suzmue commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

This removes the hacking of the build script to modify includes.rs to add the convert module. This adds a use statement to bring the crate local proto types into scope.

An unexpected benefit is that the generated code for convert.rs is now also formatted.

@suzmue
suzmue requested review from a team as code owners September 9, 2026 04:16
@suzmue

suzmue commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Regenerated code: googleapis/google-cloud-rust#6747

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 Rust code generation for hybrid crates by replacing the convert-include-package configuration with a more flexible include-file option and introducing a customizable prost-path parameter. It also cleans up the template-based code generation by removing the post-processing step in build.rs and conditionally compiling the convert module. The review feedback highlights potential syntax errors (such as double colons ::::) in the generated Rust code when the package name is empty, suggesting conditional checks in both the Go generator and the Mustache template to handle empty package names gracefully.

Comment thread internal/sidekick/rust/annotate_model.go
Comment thread internal/sidekick/rust/templates/convert-prost/convert.rs.mustache
@suzmue
suzmue enabled auto-merge (squash) September 10, 2026 23:53
@suzmue
suzmue merged commit 9914e5c into googleapis:main Sep 10, 2026
45 checks passed
@suzmue
suzmue deleted the convert-module-cleanup branch September 10, 2026 23:58
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.

2 participants