Resilience readme, namespace #4659
Conversation
Tratcher
requested review from
sebastienros,
RussKie,
joperezr,
geeknoid and
martintmk
|
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=458803&view=codecoverage-tab |
| public static IServiceCollection AddResilienceEnricher(this IServiceCollection services) | ||
| ``` | ||
|
|
||
| This will optionally consume the `IExceptionSummarizer` service if it has been registered and add that data to Polly's telemetry. It will also include `RequestMetadata` that can be set or retrieved with these extensions: |
There was a problem hiding this comment.
I wonder if the README should have a pointer to the exception summarizer package as well as the Telemetry one. That said, it probably makes more sense to point to conceptual docs describing these, so perhaps it's fine to keep as is and once we have those conceptual docs we just add a pointer to those here.
There was a problem hiding this comment.
Yeah, concept doc links would be more helpful.
| The services can be registered using the following method: | ||
|
|
||
| ```csharp | ||
| public static IServiceCollection AddResilienceEnricher(this IServiceCollection services) |
There was a problem hiding this comment.
I put a semi-colon on these API blocks. We should standardize when all the READMEs are in
There was a problem hiding this comment.
How does the semi-colon help? Neither are syntactically correct. This is a method definition which would be followed by { ... }. A semi-colon would make sense if this was a calling example like serviceCollection.AddResilienceEnricher();.
I moved the ResilienceContext extensions to match the Polly namespace.
Microsoft Reviewers: Open in CodeFlow