-
Notifications
You must be signed in to change notification settings - Fork 1.6k
8.6 Support HOTKEYS #3008
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
8.6 Support HOTKEYS #3008
Conversation
- prefer Nullable<T> when not-always-present
| /// <param name="flags">The command flags to use.</param> | ||
| /// <returns>The data captured during <c>HOTKEYS</c> profiling.</returns> | ||
| [Experimental(Experiments.Server_8_6, UrlFormat = Experiments.UrlFormat)] | ||
| HotKeysResult? HotKeysGet(CommandFlags flags = CommandFlags.None); |
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.
Must we add more synchronous methods? :(
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.
It is a great question. Historically we've kept them aligned (and we have unit tests that enforce that), but maybe we should start to rethink that. I think that's a bigger discussion than this PR, though.
Maybe step one is a custom Roslyn analyzer to start nudging people in the direction of preferring the async APIs, without using [Obsolete] or [Experimental] hackery.
HOTKEYSis a new profiling API in 8.6, allowing a server-side profiling session that shows (unsurprisingly) the "hottest" keys observed during the profiling, separately by CPU and network (which can be enabled/disabled independently).Usage is discussed in the docs as part of this PR
Note tests are linked to 8.6; CI bumped to 8.6 GA:
On files: I'm trying a new layout here; instead of making a mess throughout huge files, I've focused these changes at the "feature" scope; the API is in HotKeys.cs, with the implementation code in sub-files:
IMO this is much more friendly, and if there's no objections, I'll add "get the AI to move things around" to the backlog.