feat(sidekick/swift): bootstrap method generation#5146
feat(sidekick/swift): bootstrap method generation#5146coryan merged 4 commits intogoogleapis:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces Swift method annotation logic and updates the service template to generate method stubs. The changes include a new annotateMethod function, corresponding unit tests for name escaping, and logic to iterate through service methods during annotation. Feedback was provided regarding redundant assignments when modifying objects in-place, the need to handle streaming or long-running operations in the Swift template, and a discrepancy between the PR description and the use of fatalError instead of throwing a recoverable error.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces Swift method generation by adding method annotation logic and updating the service template. Key changes include the implementation of annotateMethod to handle method-level metadata and the inclusion of method signatures in the Swift service mustache template. Feedback was provided regarding the annotateMethod function signature to avoid ignored return values when modifying objects in-place, and a suggestion was made to use conditional logic in the template to prevent generating invalid Swift code when return types are empty.
I am having trouble creating individual review comments. Click here to see my feedback.
internal/sidekick/swift/annotate_method.go (26-34)
For consistency with annotateService in annotate_service.go, consider using codec as the receiver name instead of c. Additionally, the function currently returns a value that is ignored by the caller while modifying the method object in-place. Following repository guidelines, the function should be refactored to return nothing (or only an error) to make the side effect explicit.
func (codec *codec) annotateMethod(method *api.Method) {
docLines := codec.formatDocumentation(method.Documentation)
annotations := &methodAnnotations{
Name: camelCase(method.Name),
DocLines: docLines,
}
method.Codec = annotations
}References
- Avoid functions that return an object modified in-place if it results in the caller explicitly discarding the return value. Instead, refactor the function to return only an error to make the side effect explicit, or return the updated parent container.
internal/sidekick/swift/templates/common/service.swift.mustache (34-35)
The current template will generate invalid Swift code (a trailing -> with no type) if OutputType.Codec.Name is empty, which occurs for unsupported types like google.protobuf.Empty as mentioned in the PR description. Using a conditional block ensures the return arrow and type are only generated when a valid return type name is available.
public func {{Codec.Name}}(request: {{InputType.Codec.Name}}) async throws{{#OutputType.Codec.Name}}
-> {{.}}{{/OutputType.Codec.Name}}
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces method annotation logic for Swift code generation, including support for camel-casing and name escaping. It also updates the service template to generate asynchronous method signatures. Feedback was provided to improve the template by conditionally rendering the return arrow to avoid syntax errors when a method returns an empty response.
Generate a skeleton of each method, the skeleton just throws an exception for now. It should have the correct return type for the APIs we want to generate.
39677e6 to
5d1bdea
Compare
Generate a skeleton of each method, the skeleton just crashes the application (think
assert()) for now. It should have the correct return type for the APIs we want to generate, but does not supportgoogle.protobuf.Empty, or pagination, or LROs yet.Towards #5130