Skip to content

Conversation

@gewarren
Copy link
Contributor

Contributes to #7451.

@gewarren gewarren requested review from a team, ViktorHofer and tarekgh as code owners December 20, 2021 23:22
@ghost ghost added the area-Meta Concerns something that extends across runtime area boundaries, for example, IDisposable. label Dec 20, 2021
@opbld30

This comment has been minimized.

@opbld33
Copy link

opbld33 commented Dec 21, 2021

Docs Build status updates of commit 65074f5:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Diagnostics.Metrics/Counter`1.xml ✅Succeeded View
xml/System.Diagnostics.Metrics/Histogram`1.xml ✅Succeeded View
xml/System.Diagnostics.Metrics/Meter.xml ✅Succeeded View
xml/System.Diagnostics.Metrics/ObservableCounter`1.xml ✅Succeeded View
xml/System.Diagnostics.Metrics/ObservableGauge`1.xml ✅Succeeded View
xml/System.Diagnostics.Tracing/EventCounter.xml ✅Succeeded View
xml/System.Diagnostics.Tracing/IncrementingEventCounter.xml ✅Succeeded View
xml/System.Diagnostics.Tracing/IncrementingPollingCounter.xml ✅Succeeded View
xml/System.Diagnostics.Tracing/PollingCounter.xml ✅Succeeded View
xml/System.Diagnostics/PerformanceCounter.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

Copy link
Contributor

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

The proposed changes LGTM. I just added a missing returns, if you don't mind including it in this PR. Feel free to reword it if necessary.

BTW, the area-System.Diagnostics.PerformanceCounter does not yet exist in dotnet-api-docs, but it does exist in runtime. I'm going to create it in this repo and make sure the label automatically adds the code owners.

<Docs>
<summary>Observe() fetches the current measurements being tracked by this observable counter.</summary>
<summary>Fetches the current measurements being tracked by this observable counter.</summary>
<returns>To be added.</returns>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any objections to adding the missing returns value, since we're already here?:

Suggested change
<returns>To be added.</returns>
<returns>An enumeration of measurements of type `<typeparamref name="T"/>` that are being tracked by this observable counter.</returns>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since T isn't listed as a typeparam, presumably because it's not specified by the caller, do you think it's okay to use typeparamref?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess because this is a sealed class, these custom comments don't show up anyway. Only inherited members are shown.

https://review.docs.microsoft.com/en-us/dotnet/api/system.diagnostics.metrics.observablegauge-1?view=net-6.0&branch=pr-en-us-7510

@carlossanlop carlossanlop added area-System.Diagnostics and removed area-Meta Concerns something that extends across runtime area boundaries, for example, IDisposable. labels Dec 22, 2021
@ghost
Copy link

ghost commented Dec 22, 2021

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes to #7451.

Author: gewarren
Assignees: -
Labels:

area-System.Diagnostics

Milestone: -

@opbld33
Copy link

opbld33 commented Dec 22, 2021

Docs Build status updates of commit 00ccca4:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Diagnostics.Metrics/Counter`1.xml ✅Succeeded View
xml/System.Diagnostics.Metrics/Histogram`1.xml ✅Succeeded View
xml/System.Diagnostics.Metrics/Meter.xml ✅Succeeded View
xml/System.Diagnostics.Metrics/ObservableCounter`1.xml ✅Succeeded View
xml/System.Diagnostics.Metrics/ObservableGauge`1.xml ✅Succeeded View
xml/System.Diagnostics.Tracing/EventCounter.xml ✅Succeeded View
xml/System.Diagnostics.Tracing/IncrementingEventCounter.xml ✅Succeeded View
xml/System.Diagnostics.Tracing/IncrementingPollingCounter.xml ✅Succeeded View
xml/System.Diagnostics.Tracing/PollingCounter.xml ✅Succeeded View
xml/System.Diagnostics/PerformanceCounter.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@gewarren gewarren merged commit 2c90133 into dotnet:main Dec 23, 2021
@gewarren gewarren deleted the metrics-links branch December 23, 2021 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants