Skip to content

Add IReadOnlyDictionary to BalancerAttributes collection#2121

Merged
JamesNK merged 1 commit into
grpc:masterfrom
JamesNK:jamesnk/attributescollection-readonlydictionary
May 16, 2023
Merged

Add IReadOnlyDictionary to BalancerAttributes collection#2121
JamesNK merged 1 commit into
grpc:masterfrom
JamesNK:jamesnk/attributescollection-readonlydictionary

Conversation

@JamesNK

@JamesNK JamesNK commented May 11, 2023

Copy link
Copy Markdown
Member

Make this collection feel a little more idiomatic to use. Some extension methods in .NET hang off the readonly interface and assume all dictionaries implements IDictionary and IReadOnlyDictionary.

/// </para>
/// </summary>
public sealed class BalancerAttributes : IDictionary<string, object?>
public sealed class BalancerAttributes : IDictionary<string, object?>, IReadOnlyDictionary<string, object?>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Assuming we don't need an API review for adding this interface.

@mitchdenny mitchdenny left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM but I don't know whether this qualifies for needing an API review or not. But seems reasonable.

@JamesNK

JamesNK commented May 12, 2023

Copy link
Copy Markdown
Member Author

This type is basically a copy of HttpRequestOptions. And they've already done the API review 😄

dotnet/runtime#68149

@JamesNK JamesNK merged commit 2dc971e into grpc:master May 16, 2023
@JamesNK JamesNK deleted the jamesnk/attributescollection-readonlydictionary branch May 16, 2023 23:17
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.

3 participants