Skip to content
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

Simplify generated code #2131

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Nov 26, 2024

Motivation:

A number of things have changed since the generated code was initially designed, which means it can be simplified and improved.

Modifications:

  • As services are namespaced within a namespace-service enum (rather than within a service enum in a namespace enum), they no longer need to be grouped by namespace.
  • As deployment targets must be set in the package manifest, the generated code no longer needs to be annotated with availability guards so these have been removed.

Result:

Simpler code generation, better generated code.

Motivation:

A number of things have changed since the generated code was initially
designed, which means it can be simplified and improved.

Modifications:

- As services are namespaced within a namespace-service enum (rather
  than within a service enum in a namespace enum), they no longer need
  to be grouped by namespace.
- As deployment targets must be set in the package manifest, the
  generated code no longer needs to be annotated with availability
  guards so these have been removed.

Result:

Simpler code generation, better generated code.
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Nov 26, 2024
@glbrntt glbrntt enabled auto-merge (squash) November 26, 2024 13:19
@@ -1134,8 +1134,9 @@ struct TextBasedRenderer: RendererProtocol {
/// Renders the specified code block.
func renderCodeBlock(_ description: CodeBlock) {
if let comment = description.comment { renderComment(comment) }
let item = description.item
renderCodeBlockItem(item)
if let item = description.item {
Copy link
Collaborator

@rnro rnro Nov 26, 2024

Choose a reason for hiding this comment

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

The previous style had a cleaner call site; what was the impetus to change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

description.item is now Optional, so we need this change (or for renderCodeBlockItem to take an optional)

Copy link
Collaborator

@rnro rnro left a comment

Choose a reason for hiding this comment

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

One question otherwise it looks good

@glbrntt glbrntt merged commit d70fbad into grpc:main Nov 26, 2024
44 of 46 checks passed
@glbrntt glbrntt deleted the v2/03-simplify-generated-code branch November 26, 2024 13:33
glbrntt pushed a commit that referenced this pull request Dec 4, 2024
## Motivation
#2131 removed all usages of
availability guards. However, the `guarded` Declaration case was not
removed even though it's unused.

## Modifications
This PR removes the `guarded` case since it's not used anywhere anymore.

## Result
Cleaner codebase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants