Skip to content

Cleaned up constructors, regions and variables in NewDefaultUrlProvider#19403

Merged
kjac merged 1 commit intomainfrom
v16/improvement/new-default-url-provider-clean-up
May 26, 2025
Merged

Cleaned up constructors, regions and variables in NewDefaultUrlProvider#19403
kjac merged 1 commit intomainfrom
v16/improvement/new-default-url-provider-clean-up

Conversation

@AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Resolves: #19402 (although the complete resolution won't come until 17, we can close this after this PR is merged as there are no further immediate actions and those needed later are indicated in the obsoletion messages).

Description

This PR is just some minor refactoring and preparation for Umbraco 17 when we can finally remove the obsolete parameters in the constructor for this class. I added a new constructor using the replacement for the obsolete ILocalizationService, removed the unnecessary regions and resolved the remaining warnings in the class.

Testing

Should be clear from visual inspection there's no concern with the changes, but I have verified that URLs continue to be generated having booted up Umbraco.

Copilot AI review requested due to automatic review settings May 23, 2025 13:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the NewDefaultUrlProvider by introducing a new constructor that takes ILanguageService, removing obsolete regions and fields, and resolving compiler warnings.

  • Added a primary constructor accepting ILanguageService and marked legacy constructors as obsolete for removal in v17
  • Removed unused _localizationService field, cleaned up region directives, and simplified string slicing
  • Replaced synchronous ILocalizationService calls, updated explicit type annotations, and streamlined logging checks
Comments suppressed due to low confidence (1)

src/Umbraco.Core/Routing/NewDefaultUrlProvider.cs:83

  • The logger is declared as ILogger<DefaultUrlProvider> inside NewDefaultUrlProvider. Consider using ILogger<NewDefaultUrlProvider> to ensure logged messages use the correct category name.
ILogger<DefaultUrlProvider> logger,

@AndyButland AndyButland changed the title Cleaned up constructors, regions and variables in NewDefaultUrlProvider. Cleaned up constructors, regions and variables in NewDefaultUrlProvider May 23, 2025
Copy link
Contributor

@kjac kjac left a comment

Choose a reason for hiding this comment

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

Looks good and tests out good too 👍

@kjac kjac merged commit 13e368c into main May 26, 2025
25 of 26 checks passed
@kjac kjac deleted the v16/improvement/new-default-url-provider-clean-up branch May 26, 2025 05:11
AndyButland added a commit that referenced this pull request May 26, 2025
…er (#19403)

Cleaned up constructors, regions and variables in NewDefaultUrlProvider.
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.

NewDefaultUrlProvider using obsolete ILocalizationService

3 participants