dotnet / Scaffolding

Code generators to speed up development.
MIT License
635 stars 227 forks source link

Update Web API scaffolders to use new Web API conventions #636

Closed danroth27 closed 6 years ago

danroth27 commented 6 years ago

We are adding some new [Web API specific conventions]() that we will need the Web API scaffolders to react to.

We are adding [ApiController] attribute as the way to opt-in to new API specific conventions and behaviors that are tuned around best practices for APIs.

This will include:

danroth27 commented 6 years ago

@vijayrkn What's the status of this issue? Looks like it was never assigned to anyone.

vijayrkn commented 6 years ago

I have assigned it to Sean but I don't think we will be able to get this fixed in 2.1.0-rc1. Can we move this out to the next release?

danroth27 commented 6 years ago

I'd like to get it done for 2.1.0 RTW. We've already made these updates for the project templates.

vijayrkn commented 6 years ago

We have a few more scaffolding items to finish for rc1.
@seancpeters Can you please take a look at this once you are done with all the rc1 items?

seancpeters commented 6 years ago

@danroth27 - I have some time to work on this issue now. Do you know the PR for the corresponding template changes? If I understand correctly, this work would be to apply similar changes to the scaffolding content.

danroth27 commented 6 years ago

@seancpeters I think it was this one: https://github.com/aspnet/templating/pull/220

seancpeters commented 6 years ago

@danroth27 - I noticed that in the aspnet/templating, not all controllers were changed to using ControllerBase (see below). With this in mind, should I make the change to all of the scaffolded Controllers? I'm thinking yes.

Only the WebApi-CSharp template has the change: https://github.com/aspnet/templating/blob/dev/src/Microsoft.DotNet.Web.ProjectTemplates/content/WebApi-CSharp/Controllers/ValuesController.cs

These do not: StarterWeb: https://github.com/aspnet/templating/blob/dev/src/Microsoft.DotNet.Web.ProjectTemplates/content/StarterWeb-CSharp/Controllers/HomeController.cs Angular-CSharp: https://github.com/aspnet/templating/blob/release/2.1/src/Microsoft.DotNet.Web.Spa.ProjectTemplates/content/Angular-CSharp/Controllers/SampleDataController.cs React-CSharp: https://github.com/aspnet/templating/blob/release/2.1/src/Microsoft.DotNet.Web.Spa.ProjectTemplates/content/React-CSharp/Controllers/SampleDataController.cs ReactRedux-CSharp https://github.com/aspnet/templating/blob/release/2.1/src/Microsoft.DotNet.Web.Spa.ProjectTemplates/content/ReactRedux-CSharp/Controllers/SampleDataController.cs

danroth27 commented 6 years ago

@seancpeters Only web API controllers should derive from ControllerBase. Web UI controllers (like HomeController) should continue to derive from Controller. Only change Web API specific scaffolders to derive from ControllerBase.

Looks like we missed updating the SPA templates to use the new Web API conventions. We'll have to deal with that in the next release.

muratg commented 6 years ago

@vijayrkn What's the status of this? This was approved 4 days ago.

vijayrkn commented 6 years ago

@seancpeters Is this already merged?

seancpeters commented 6 years ago

@vijayrkn @muratg - yes, the corresponding PR #772 was merged on Tuesday I believe.

So I'll close this issue.