-
Notifications
You must be signed in to change notification settings - Fork 6k
ASP.NET Built-in Metrics: Delete and forward to new location #45791
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.
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
base: main
Are you sure you want to change the base?
Conversation
Closing and reopening to attempt re-build since the OPS status checker seems to be having an existential crisis, absorbed in an endless loop of self-reflection... |
@@ -403,7 +403,7 @@ items: | |||
- name: .NET extensions metrics | |||
href: ../../core/diagnostics/built-in-metrics-diagnostics.md | |||
- name: ASP.NET Core metrics | |||
href: ../../core/diagnostics/built-in-metrics-aspnetcore.md | |||
href: /aspnet/core/log-mon/metrics/built-in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make this a contextual TOC link? TOC links aren't supposed to take you away from the current TOC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gewarren, good point. If by "contextual TOC link" you mean linking to a topic in this repository first so that the user then hits a link again in that topic to go to the eventual topic in the other repo, I guess that would either be a link to the Metrics > Overview just above this one (which feels a little too circular), or creating a new topic in this repo just to have a link to the actual topic (which we also avoid unless we really have to.)
It looks like to get around this problem for the EF metrics, that the EF metrics link was not provided in the TOC and the Metrics > Overview was just relied on where it has a link to the metric topic in the EF repo. So, I will go that route unless there is something you feel that was better to do.
@@ -402,8 +402,6 @@ items: | |||
href: ../../core/diagnostics/built-in-metrics-runtime.md | |||
- name: .NET extensions metrics | |||
href: ../../core/diagnostics/built-in-metrics-diagnostics.md | |||
- name: ASP.NET Core metrics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the ASP.NET Core metrics link to rely on The Build-in metrics > Overview link instead in the TOC, as the EF metrics does, to avoid linking directly to the metrics topic in a different repo and TOC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep this TOC entry. The contextual TOC link should look like this:
href: /aspnet/core/log-mon/metrics/built-in?toc=/dotnet/navigate/tools-diagnostics/toc.json&bc=/dotnet/breadcrumb/toc.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a suggestion with how a contextual TOC link works (you can also read about them in the contributor guide).
OH! You wanted a breadcrumb! Sorry, I was not catching on. Thanks! |
Co-authored-by: Genevieve Warren <[email protected]>
Summary
Fixes #45580
Fixes dotnet/AspNetCore.Docs#35109
Internal previews