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.

Sign up for GitHub

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

Jump to bottom

Self-Review Metrics draft #3

Closed
wants to merge 41 commits into from
Closed

Self-Review Metrics draft #3

wants to merge 41 commits into from

Conversation

Tratcher
Copy link
Owner

@Tratcher Tratcher commented Jul 19, 2023 •
edited
Loading

@Tratcher Tratcher self-assigned this Jul 19, 2023
noahfalk
noahfalk reviewed Jul 20, 2023
View reviewed changes
Copy link

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

I didn't go into deep detail, but everything here looked good to me so far. Thanks!

src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/README.md Outdated Show resolved Hide resolved
...Extensions.Diagnostics.Abstractions/src/Microsoft.Extensions.Diagnostics.Abstractions.csproj Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics.Configuration/README.md Outdated Show resolved Hide resolved
noahfalk
noahfalk reviewed Jul 25, 2023
View reviewed changes
Copy link

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Some suggestions inline for you, overall I'm very happy 👍

src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ConsoleMetricListener.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/MetricsBuilderConsoleExtensions.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Diagnostics/tests/ConsoleMetricListenerTests.cs Outdated Show resolved Hide resolved
src/libraries/Microsoft.Extensions.Hosting/src/HostApplicationBuilder.cs Show resolved Hide resolved
@Tratcher
Copy link
Owner Author

Tratcher commented Aug 9, 2023

Replaced by dotnet#90201

@Tratcher Tratcher closed this Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noahfalk noahfalk noahfalk left review comments

@tarekgh tarekgh tarekgh left review comments

@CodeBlanch CodeBlanch CodeBlanch left review comments

Assignees

@Tratcher Tratcher

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Tratcher @noahfalk @tarekgh @CodeBlanch