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.

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

[WIP] Obsoleting Pseudo-header public APIs #42199

Conversation

Daniel-Genkin-MS-2
Copy link
Contributor

Currently we place the pseudo-headers in the HeaderNames.cs file which exposes them as public API. This is redundant as the pseudo-headers are all stripped out in internal code prior to passing it up to publicly available layers. Thus, these headers cannot actually be used by end users. This PR marks them as obsolete, removes them from intellisense and adds them to an internal-only file, PseudoHeaderNames.cs.

@@ -61,6 +63,8 @@ public static class HeaderNames
public static readonly string AltSvc = "Alt-Svc";

/// <summary>Gets the <c>:authority</c> HTTP header name.</summary>
[Obsolete("For infrastructure use only", false)]
[EditorBrowsable(EditorBrowsableState.Never)]
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't hide it from intellisense. That's one of the most annoying experiences as a developer when you see it in the docs and then it doesn't show up in the IDE. Obsoletion is enough

@Daniel-Genkin-MS-2
Copy link
Contributor Author

Closing as I found a way more elegant solution than trying to fix this cyclic dependency

#42496

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants