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

Cleanup deprecated code and TODOs #1457

Merged
merged 4 commits into from
Nov 3, 2023
Merged

Cleanup deprecated code and TODOs #1457

merged 4 commits into from
Nov 3, 2023

Conversation

josephdecock
Copy link
Member

@josephdecock josephdecock commented Nov 1, 2023

  • Remove code marked as unused/to be removed in 7.0
  • Fix typo in name
  • Clean up some very old todos that don't need to be done anymore

- Remove code marked as unused/to be removed in 7.0
- Fix typo in name
- Clean up some very old todos that don't need to be done anymore
@josephdecock josephdecock added this to the 7.0.0 milestone Nov 1, 2023
@josephdecock josephdecock marked this pull request as draft November 1, 2023 20:10
@josephdecock josephdecock marked this pull request as ready for review November 1, 2023 20:35
@@ -6,6 +6,6 @@ namespace Duende.IdentityServer;

internal static class Constants
{
public const string IdentityServerName = "IdentityServer4";
public const string IdentityServerName = "Duende.IdentityServer";
Copy link
Member

Choose a reason for hiding this comment

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

where is this used? will this change cause any breaking changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this is likely to break anyone. It is used

  • As the default value for the CorsPolicyName option and
  • As the value for IdentityServerAuthenticationType

I guess theoretically if someone has created a cors policy and named it "Duende.IdentityServer", this conflicts with that. That seems pretty unlikely to me.

And theoretically, a customer could have a customization that cares about the authentication type and refers to it without using this constant. Again, seems pretty unlikely.

OTOH, this is a low-value change.

@brockallen brockallen merged commit 90a75b8 into main Nov 3, 2023
5 checks passed
@brockallen brockallen deleted the joe/remove-deprecated branch November 3, 2023 12:58
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.

2 participants