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

Review the list of obsolete types and see whether we can remove any of these in .NET 6 #27529

Closed
mkArtakMSFT opened this issue Nov 4, 2020 · 9 comments
Labels
area-hosting Includes Hosting area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-razor.compiler This issue is related to the Razor compiler (now external) area-signalr Includes: SignalR clients and servers design-proposal This issue represents a design proposal for a different issue, linked in the description feature-azure Issues related to integration with Azure components (Azure App Service Logging, App Insights, etc.) Theme: meeting developer expectations
Milestone

Comments

@mkArtakMSFT
Copy link
Member

mkArtakMSFT commented Nov 4, 2020

For .NET 6, we'd like to evaluate the APIs marked as obsolete and remove them if necessary. There's ~130 APIs marked as obsolete that are split up based on project area in the table below. The goal is to have these APIs removed by preview1 so we have time to get feedback on our decision early on in the release.

What to do

  • If it should be removed
    • Remove it
    • Update any samples/tests that use the obsolete API.
  • If it should not be removed:
    • Add an inline comment explaining why and provide a timeline for removal if possible
    • Ensure that the Obsolete description provides information about alternative APIs or guidelines on what the user should do otherwise.
    • Provide an aka.ms link to the deprecation announcement.

Who is doing it

Area Assigned Done?
Azure AD @HaoK, @JunTaoLuo, @javiercn #27955
Hosting @Tratcher, @halter73 #27529 (comment)
Http @Tratcher, @jkotalik , @halter73 #28890
Middleware @Tratcher, @HaoK, @halter73 #28893
MVC @pranavkm, @javiercn, @captainsafia #28337
Razor @ajaybhargavb, @captainsafia #28706
Servers @jkotalik, @halter73 #29275
SignalR @BrennanConroy #28100

Things to keep in mind
@halter73 and @captainsafia took a look at all the APIs and made the following list of things to consider when evaluating whether or not an Obsoleted API should be removed.

  • Outside of redundancy, is there another harm to having the API around? For example, it is not performant or not supported by other parts of the ecosystem.
  • What's the maintenance cost of keeping the API around? This includes things like the number of issues reported by users, whether there's a lot of flaky tests associated with the API, etc.
  • Does the Obsoleted API no longer work?
  • How common is the API that is being obsoleted? Is it likely a new user will inadvertently use it and run into issues?
  • Is it actually a shipped API?
  • When was the API obsoleted? If it was obsoleted in .NET 5, keep it around to make upgrading between LTS releases, from 3.1 to 6, easier for devs.
  • Is this API commonly used by other packages?
@mkArtakMSFT mkArtakMSFT added design-proposal This issue represents a design proposal for a different issue, linked in the description Theme: meeting developer expectations labels Nov 4, 2020
@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone Nov 4, 2020
@ghost
Copy link

ghost commented Nov 4, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@Pilchie
Copy link
Member

Pilchie commented Nov 17, 2020

Also to keep in mind - if the API was first marked Obsolete in .NET 5.0, we might consider keeping it for .NET 6 to make it easier for people to upgrade from LTS to LTS (.NET Core 3.1 -> .NET 6.0).

@javiercn
Copy link
Member

We should consider adding aka.ms links to the obsolete message that point to a landing doc (the announcement?) and that suggest alternatives.

@dougbu
Copy link
Member

dougbu commented Nov 18, 2020

We may also want to use the new [Obsolete] DiagnosticId property. The dotnet/runtime team is using this extensively. See https://github.com/dotnet/runtime/blob/329f0dbfa3a164ee2e3bf9f75b7641165995c799/src/libraries/Common/src/System/Obsoletions.cs and e.g. https://github.com/dotnet/runtime/blob/329f0dbfa3a164ee2e3bf9f75b7641165995c799/src/libraries/System.Security.Permissions/src/System/Security/SecurityManager.cs#L10

@captainsafia
Copy link
Member

@dougbu Nice suggestion. I think that's something we should do that once we've gathered info about all the obsoleted APIs. I've updated the issue comment to reflect adding aka.ms links when necessary.

@Pilchie Pilchie added feature-azure Issues related to integration with Azure components (Azure App Service Logging, App Insights, etc.) area-hosting area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-razor.compiler This issue is related to the Razor compiler (now external) area-signalr Includes: SignalR clients and servers labels Dec 15, 2020
@Tratcher
Copy link
Member

Tratcher commented Dec 18, 2020

Obsolete Hosting types:

  • EnvironmentName: Constants that have been moved to Extensions with a new name but otherwise unchanged. Leave it.
  • IApplicationLifetime: An interface that has been moved to Extensions with a new name but otherwise unchanged. Leave it.
  • IHostingEnvironment: An interface that has been moved to Extensions with a new name but otherwise unchanged. Leave it.

All include comments about their recommended replacements. I don't think any additional action is needed here. I'll mark it as done.

@halter73
Copy link
Member

@mkArtakMSFT @captainsafia Can we close this now?

@captainsafia
Copy link
Member

This PR is still open: #27955

But it's being tracked so we can close this.

We may want to move #27529 (comment) out into a separate issue as well.

But yes, this work is mostly done for preview1. Thanks everyone for chipping in!

@BrennanConroy
Copy link
Member

FYI, created aspnet/Announcements#450 for all PRs linked in the first comment, except for the unmerged AzureAD ones.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 27, 2021
@amcasey amcasey added area-hosting Includes Hosting area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware and removed area-runtime labels Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-hosting Includes Hosting area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-razor.compiler This issue is related to the Razor compiler (now external) area-signalr Includes: SignalR clients and servers design-proposal This issue represents a design proposal for a different issue, linked in the description feature-azure Issues related to integration with Azure components (Azure App Service Logging, App Insights, etc.) Theme: meeting developer expectations
Projects
None yet
Development

No branches or pull requests

14 participants