Fix How we Reference Generic Types in Doc-Comments#4585
Merged
InsertCreativityHere merged 4 commits intoMay 7, 2026
Merged
Conversation
The correct syntax is to use '{' and '}' instead of '<' and '>', even if
they're XML escaped to '>' or '<'.
There was a problem hiding this comment.
Pull request overview
This PR updates internal XML doc-comments to use the {} generic-parameter syntax rather than <> (even when XML-escaped), based on the findings from #4566/#4494 about cref formatting.
Changes:
- Updated doc-comment text in the Slice generators to reference generic types using
{}. - Adjusted summaries for parameter/return type mapping helpers to align with the new generic-type reference style.
- Updated a doc-comment in
FieldExtensionsdescribingReadOnlyMemorymapping.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/ZeroC.Slice.Generator/TypeRefExtensions.cs | Updates summaries describing incoming/outgoing parameter type mappings for sequences/dictionaries. |
| src/ZeroC.Slice.Generator/FieldExtensions.cs | Updates summary describing ReadOnlyMemory<T> mapping for outgoing parameters. |
| src/IceRpc.Slice.Generator/OperationExtensions.cs | Updates summary describing operation return types (Task, Task<T>, tuple). |
pepone
reviewed
May 7, 2026
| } | ||
|
|
||
| /// <summary>Returns the C# return type for an operation (Task, Task<T>, or Task<tuple>). | ||
| /// <summary>Returns the C# return type for an operation (Task, Task{T}, or Task{tuple}). |
Member
There was a problem hiding this comment.
We should use the code tag for those.
|
|
||
| /// <summary>Returns the C# type string for an incoming parameter (decode target). Sequences map to arrays, | ||
| /// dictionaries map to Dictionary<K,V>. Respects cs::type attribute.</summary> | ||
| /// dictionaries map to Dictionary{K,V};. Respects cs::type attribute.</summary> |
Member
There was a problem hiding this comment.
Dictionary and cs::type should be render inside code tag IMO
| /// primitives map to ReadOnlyMemory<T>, other sequences to IEnumerable<T>, dictionaries to | ||
| /// IEnumerable<KeyValuePair<K,V>>.</summary> | ||
| /// primitives map to ReadOnlyMemory{T}, other sequences to IEnumerable{T}, dictionaries to | ||
| /// IEnumerable{KeyValuePair{K,V}}.</summary> |
Member
There was a problem hiding this comment.
Ditto use code tag for the code bits
Member
Author
|
Okay, I wrapped all of the generics in code-tags, instead of using the curly-brace syntax. |
| /// <value>A pool of memory blocks used for buffer management. Defaults to <see cref="MemoryPool{T}.Shared" | ||
| /// />.</value> | ||
| /// <remarks>The built-in TCP transport requires the pool to return array-backed memory blocks, because its | ||
| /// multi-segment write path uses <see cref="System.Net.Sockets.Socket.SendAsync(System.Collections.Generic.IList{ArraySegment{byte}}, |
Member
Author
There was a problem hiding this comment.
While looking through everywhere we reference a generic type with curly braces (to catch any others that should use code-formatting) I came across a couple places which the IDE said were over-qualified.
pepone
approved these changes
May 7, 2026
bernardnormier
approved these changes
May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We should use code-formatting for the places which are actually code, instead of leaving them in plain-text formatting.
Old, and no longer relevant:
The correct syntax is to use '{' and '}' instead of '<' and '>', even if they're XML escaped to '>' or '<'.
As we figured out after some research on #4566.