feat: Added helper methods to Google.Rpc.Status for unpacking the Detail #736
Conversation
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
|
Will try to review this later today. |
tonydnewell
changed the title
There was a problem hiding this comment.
This is basically fine, thanks - various style things which are different in our repo, and much easier for me to pick up after submission. Will leave until tomorrow morning UK time just in case my comments suggest any other changes, but otherwise I'll merge and then work with @amanda-tarafa on little style things - then we can release...
| using System.Collections.Generic; | ||
| using Xunit; | ||
|
|
||
| namespace Google.Api.CommonProtos.Tests |
There was a problem hiding this comment.
For new code, please use file-scoped namespaces.
| var badRequest = new BadRequest() | ||
| { | ||
| FieldViolations = | ||
| { |
There was a problem hiding this comment.
(Although I'm happy to basically go over everything later for style.)
|
|
||
| /// <summary> | ||
| /// Get the registry | ||
| /// Note: experimental API that can change or be removed without any prior notice. |
There was a problem hiding this comment.
We don't use this sort of note in our libraries. Once it's there, it's there.
| /// <typeparam name="T"></typeparam> | ||
| private static class MessageNameCache<T> where T : class, IMessage<T>, new() | ||
| { | ||
| public static readonly string FullName = new T().Descriptor.FullName; |
There was a problem hiding this comment.
I know this is private to the class, but let's use a property anyway - just add { get; }
| { | ||
| return null; | ||
| } | ||
| return any.Unpack<T>(); |
There was a problem hiding this comment.
Just return any?.Unpack<T>(); and avoid the check above? (Indeed, could do this without the any variable at all.)
| foreach (var any in Details) | ||
| { | ||
| var msg = any.Unpack(registry); | ||
| if (!(msg is null)) |
There was a problem hiding this comment.
Perhaps just use an expression bodied member of Details.Select(any => any.Unpack(registry)).OfType<IMessage>());?
| } | ||
|
|
||
| [Fact] | ||
| public void UnpackDetailMessageTest() |
There was a problem hiding this comment.
I fully expect to refactor this later :)
Add methods to
Google.Rpc.Statusto help with unpacking theDetailin theStatus.These methods have been moved from the proposed Grpc.StatusProto API in
grpc/grpc-dotnet#2205
@jskeet for review