-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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
Review of changes to ANCM Configuration Reference #2206
Comments
@guardrex OK |
If you like any of these, I'll get a PR together this weekend. I numbered them for reference. 1
I recommend "allows Kestrel to use" (more formal) over "lets Kestrel use" (somewhat informal). 2
Throughout .NET Core docs and samples, Also, this section doesn't strike me as really addressing 3
I'm surprised this was added at this time, as it seems to apply to VS2017, MSBuild/csproj, and utility provided by Would you like this refactored to address the use of Side note: This section has been introduced here for the first time. Since we've seen a number of devs include the 4
... change "for which" to "that" ...
5
There are three env vars set now, so we'd probably remove the I don't have a hard suggestion ... just a question on this: It seems a little strange to me to show the Would it make more sense right now to use 6
to
7
... and explicitly specify that this example is for Azure Apps. |
Um. Can I just say I find that confusing, it seems like passive voice. Kestrel is designed to be fast, not designed to be secure, or load balanced, or to have policies or management or lots of logging. So IIS is a production web hosting platform, and kestrel is an ultra lightweight http request server. So I would say "ASP.NET Core Module is the module that lets you run portable .net core applications behind IIS, using IIS for what it's good at (security, manageability, and lots more) and using Kestrel for what it's good at (being really really fast), and getting the benefits from both technologies at once". Users don't actually know or care what a Reverse Proxy Server is. Why bother explaining that at the introductory material? This is the thing that let's you have peanut butter and chocolate, and lets them be friends. |
Technically tho, the description is correct. This is merely the reference doc. I think the intention is that general info, descriptions, how-to is all in the new ANCM overview doc at https://docs.microsoft.com/en-us/aspnet/core/fundamentals/servers/aspnet-core-module. |
Oh sure, I'm just giving an End User perspective here. What I first found confusing about ANCM is that it exists at all. Then I realized, this is a more modern architecture than ISAPI or DCOM, so of course it's a reverse proxy. |
@Rick-Anderson 👍 ... but on specifics ... RE: 3 ... VS modding the RE: 5 ... the env var example: Leave it alone? ... change it to ☝️ or do you want me to make the call? In which case, I'll go with @wpostma I recommend you float that over to @tdykstra for https://docs.microsoft.com/en-us/aspnet/core/fundamentals/servers/aspnet-core-module? @Rick-Anderson, what we could do here is just drop the first sentence entirely. This whole doc can just start with the second sentence. Although, I would combine it with the next line. Like Captain Kirk, "I don't believe in the one-line paragraph." 😄 [well ... I try to avoid them most of the time] I would also make a few other minor course corrections, but this doc would open well with something like ...
[ 🎤 drop ] 😄 |
That's a good page. Why not link to that page from here? Links are awesome. People will google and land all over your docs. |
@guardrex you make the call. |
ASPNETCORE_ENVIRONMENT was my suggestion. It's a real value that people ask how to set. They're likely to set this after publishing as a way to help them debug their app. |
For the fundamentals doc, @wpostma intro sentence suggestion sounds good -- I'd just make minor changes:
For this doc I like @guardrex suggestion
Regarding "allow" vs. "lets", the choice of the latter results from an effort on Microsoft's part to make its docs sound less formal. I was accustomed to using "allow" in cases like this but have been advised that the "lets" language is preferred for that reason. Suggestions 2, 4, 6, 7 all sound fine |
@Rick-Anderson Also, curious how I can view the build progress as before? There's no link in the vicinity of ... [EDIT] I resubmitted the PR's. They went through this time. |
@guardrex sometimes the OPS build doesn't show up. It usually shows up. If you see that again, let me know. I usually close/reopen and that fixes it. |
@Rick-Anderson @spboyer @Tratcher @pranavkm @shirhatti @moozzyk @danroth27
A significant update was just made to the ASP.NET Core Module Configuration Reference to accommodate the new ANCM overview doc, but I see just a handful of very minor grammatical issues.
Instead of submitting a PR outright, I'll post on this issue the handful of suggestions that I have for approval if that works for you.
The text was updated successfully, but these errors were encountered: