-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Clean up obsolete types in Razor #28418
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
Conversation
@NTaylorMullen, do you recall whether we still use these for anything or can they be safely removed? Context here
@NTaylorMullen, this was obsoleted around April 2019. Can we safely remove it now but do you think we should wait for another release? I'm not sure if this is something customers will be depending on. |
Hmmm you can validate to see if they're consumed at all in the Mvc 1_x / 2_x projects: https://github.com/dotnet/aspnetcore/tree/master/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X
Actually I think I was stupid for obsoleting this at all. I'd actually say it's probably worth "unobsoleting" it. There's still value. |
Fair point. Un-obsoleted.
There's not consumed there. Just the Razor tests. I guess that means safe to remove? |
Yup |
Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:
|
|
Closing in favor of #28706. |
Hi @captainsafia. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
Part of #27529.
Summary of Changes
IRazorEngineBuilder
references and tests since it has been obsoleted for a while.RazorEngine
takes dependency on it.RazorEngine
type.GetItem
since it was recently obsoleted.