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

Add documentation to generated code #2133

Merged
merged 2 commits into from
Nov 27, 2024
Merged

Add documentation to generated code #2133

merged 2 commits into from
Nov 27, 2024

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Nov 27, 2024

Motivation:

The generated code doesn't consistently include documentation from the source IDL. It also doesn't explain the difference between each of the generated protocols and when to use them.

Modifications:

Add documentation to the generated code, and where possible, also include any documentation from the source IDL.

Result:

Better documented generated code

Motivation:

The generated code doesn't consistently include documentation from the
source IDL. It also doesn't explain the difference between each of the
generated protocols and when to use them.

Modifications:

Add documentation to the generated code, and where possible, also
include any documentation from the source IDL.

Result:

Better documented generated code
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Nov 27, 2024

private func explodedDocs(for method: MethodDescriptor) -> String {
let summary = "/// Call the \"\(method.name.base)\" method."
var parameters = """
Copy link
Collaborator

Choose a reason for hiding this comment

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

How often is this code called? Often enough to want to use something more efficient than +=?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Infrequently, it's only used at code generation time so not really concerned about this.

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.

This looks good and like a really good usability improvement overall 🙂

@glbrntt glbrntt enabled auto-merge (squash) November 27, 2024 13:59
@glbrntt glbrntt merged commit 311486e into grpc:main Nov 27, 2024
43 of 45 checks passed
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