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

update WebApi to .NET 9 (without Swagger) #34746

Merged
merged 18 commits into from
Feb 20, 2025

Conversation

timdeschryver
Copy link
Contributor

@timdeschryver timdeschryver commented Feb 17, 2025

Fixes #34524

This PR:

  • updates example to .NET 9, including NuGet packages
    • migrates to file-scoped namespaces to keep things consistent
  • add swagger example to test with VSCode (similar to the minimal web api docs at https://learn.microsoft.com/en-us/aspnet/core/tutorials/min-web-api?view=aspnetcore-9.0&tabs=visual-studio-code)
  • create and use new screenshots because of some small differences
  • update docs to reference .NET 9 standard support
  • update docs to use new sample and screenshots
    • update code reference import style from [!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":::
  • remove swashbuckle references and update with http files (Visual Studio) and NSwag Swagger (Visual Studio code)

Review tip: because of the namespaces it can be easier to hide whitespace changes.


Internal previews

📄 File 🔗 Preview link
aspnetcore/tutorials/first-web-api.md Tutorial: Create a controller-based web API with ASP.NET Core

@guardrex guardrex requested a review from wadepickett February 18, 2025 11:56
@wadepickett
Copy link
Contributor

wadepickett commented Feb 18, 2025

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.

@Rick-Anderson
Copy link
Contributor

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?
I can get the review on this finished today and have it merged and live tonight.

@wadepickett
Copy link
Contributor

@Rick-Anderson, closed the other one.

@timdeschryver
Copy link
Contributor Author

No problem.
Sorry, I didn't see the other PR.

@wadepickett wadepickett removed their request for review February 19, 2025 17:08
@timdeschryver
Copy link
Contributor Author

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.

Copy link
Contributor

@wadepickett wadepickett Feb 19, 2025

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.

Copy link
Contributor Author

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!

@Rick-Anderson
Copy link
Contributor

@Rick-Anderson, closed the other one.

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.
@wadepickett
Copy link
Contributor

wadepickett commented Feb 20, 2025

@Rick-Anderson, closed the other one.

Who closed what? #34676 is still open. Need to close one of them.

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.

@wadepickett wadepickett self-requested a review February 20, 2025 00:33
Comment on lines +248 to +252
* Run the following command:

```dotnetcli
dotnet add package NSwag.AspNetCore
```
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor

@wadepickett wadepickett left a 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.

@Rick-Anderson
Copy link
Contributor

@timdeschryver see my commits, I think you had an errant VS/VSC block which I removed.

Public review of the doc build:

  • download the HTML files
  • unzip
  • view HTML file in a browser.
  • For links outside the doc, remove the review. from URL.

Copy link
Contributor

@Rick-Anderson Rick-Anderson left a 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.

Copy link
Contributor

@Rick-Anderson Rick-Anderson left a 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.

Copy link
Contributor

@Rick-Anderson Rick-Anderson Feb 20, 2025

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?

Suggested change
For an alternative testing approach, select the Visual Studio Code tab and use the CLI to perform the steps.

Copy link
Contributor

@wadepickett wadepickett Feb 20, 2025

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.

Copy link
Contributor

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?

Copy link
Contributor

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"

Copy link
Contributor Author

@timdeschryver timdeschryver Feb 20, 2025

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!)

@wadepickett wadepickett self-requested a review February 20, 2025 18:59
Copy link
Contributor

@wadepickett wadepickett left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@wadepickett wadepickett self-assigned this Feb 20, 2025
@wadepickett wadepickett merged commit 517d959 into dotnet:main Feb 20, 2025
3 checks passed
@timdeschryver timdeschryver deleted the issues/34524 branch February 21, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Controller-based API" tutorial expects Swagger, but swagger isn't present by default
3 participants