-
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
update WebApi to .NET 9 (without Swagger) #34746
Conversation
I'm so sorry @timdeschryver, I was already in the middle of updating this topic after another community member started a PR #34676 on it more than a week ago. It went through a review along with more updates which I am currently dropping in for that PR. Looking back I see you asked about updating this topic and I missed it!!! I will look this over but, it has already been done, so this one will likely be closed. |
This one is ready, why don't we take it and apologize to #34676? |
@Rick-Anderson, closed the other one. |
No problem. |
I went over the other PR, and applied minor changes (2edb359) within this PR. Looking at the comments, I also want to highlight I added Visual Studio instructions using http files, and Visual Studio Code examples using Swagger (with NSwag.AspNetCore). These samples are using a different project. I based this on the Minimal API docs, which is also created with this structure. |
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.
This screenshot does not get used in the update. (It also shows the wrong project template, Empty instead of ASP.NET Core web API). I think we don't really need the screenshot anyway for this tutorial so it could just be deleted.
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.
Good catch, I must have have copy-pasted this from the previous docs and forgot to remove it.
Thanks for the review!
Who closed what? #34676 is still open. Need to close one of them. |
Control-click on a project in VSC on windows does not work, must be Right-click for Windows and Control-click on macOS for VSC.
I re-opened for a moment to get a build to check against and left a note there indicating so. Closing it in a minute. |
* Run the following command: | ||
|
||
```dotnetcli | ||
dotnet add package NSwag.AspNetCore | ||
``` |
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 this is where you need # [Visual Studio Code](#tab/visual-studio-code)
and VSC
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.
@Rick-Anderson, NSwag/Swagger setup is only being used for VSC here. The VSC tab is set currently for the entire section here: ## Create API testing UI with Swagger
That seems right the way the tabs are set here. Maybe I don't understand what you are referring to.
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.
@timdeschryver, excellent work. Thanks also for adding in my changes from dupe PR. I just made some minor changes here. See my comment on removing the New Project screenshot (_static/9/create-new-project-empty-vs17.11.0.png) and Rick's notes.
@timdeschryver see my commits, I think you had an errant VS/VSC block which I removed. Public review of the doc build:
|
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'm going to merge this once you have a look at the build @timdeschryver
I'll then publish live and you can take a closer look.
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.
revert comment out VSC
# [Visual Studio](#tab/visual-studio) --> | ||
|
||
This tutorial uses [Endpoints Explorer and .http files](xref:test/http-files#use-endpoints-explorer) to test the API. | ||
|
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.
@timdeschryver @wadepickett the change was so abrupt I thought it was a mistake. How about a suggestion to Create API testing UI with Swagger?
For an alternative testing approach, select the Visual Studio Code tab and use the CLI to perform the steps. | |
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.
Do you mean to write "Select the Visual Studio Code" tab? They are already on the VS tab at this point. I think it would make things a little confusing. They could get through it, but it feels a bit awkward.
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.
Do you mean to write "Select the Visual Studio Code" tab? They are already on the VS tab at this point. I think it would make things a little confusing. They could get through it, but it feels a bit awkward.
Yes, forgot Code,
so there's no value in VSC customers doing that section?
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.
There's value, minus a bit of awkward. I ran through at that point from VS to VS Code instruction, it works. Consider making it clear that the Swagger option is instead. Also cap "Swagger"
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.
To be honest, I also find the current wording/format unclear.
If we change this, we should also re-evaluate the minimal api docs (https://learn.microsoft.com/en-us/aspnet/core/tutorials/min-web-api?view=aspnetcore-9.0&tabs=visual-studio-code#add-the-api-code), which uses the same format.
I believe the reasoning behind the current format is that Visual Studio supports http files by default, and thus swagger isn't required.
For Visual Studio Code this is not the case, an extension needs to be installed to use http files. That's why I assume the Swagger setup is added to the Visual Studio Code tabs.
I like the suggestion from @Rick-Anderson .
An alternative is to remove the Visual Studio/Visual Studio Code setup and suggest users to install the http files extension, or point them to the "Test with other tools" section for alternatives.
If I need to update something feel free to add it here.
(Thanks for the thorough review!)
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.
Awesome, thanks!
Fixes #34524
This PR:
[!code-csharp[](~/tutorials/first-web-api/samples/9.0/TodoApi/Models/TodoItem.cs)]
to:::code language="csharp" source="~/tutorials/first-web-api/samples/9.0/TodoApi_SwaggerVersion/Models/TodoItem.cs":::
Review tip: because of the namespaces it can be easier to hide whitespace changes.
Internal previews