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

Change GetAbsoluteLoggingPath to extension method #13571

Merged
merged 1 commit into from
Dec 13, 2022

Conversation

ronaldbarendse
Copy link
Contributor

@ronaldbarendse ronaldbarendse commented Dec 13, 2022

Prerequisites

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

Description

While changing the JSON schema generation to a .NET tool that loads an assembly to get the type to generate the schema from (see PR #13560), I encountered the following error:

Could not load file or assembly 'Microsoft.Extensions.Hosting.Abstractions, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'. The system cannot find the file specified.

I'm not a 100% sure why it can't find this assembly, since Umbraco.Core does have a dependency on Microsoft.Extensions.Hosting.Abstractions 7.0.0, but I see the LoggingSettings class does use the IHostEnvironment interface from this assembly since a recent change in PR #13485.

Since these changes aren't released yet and the method using IHostEnvironment doesn't have to be on the settings class (and shouldn't be to make it a POCO), I've moved it into an extension method. This change is already included in PR #13560, but that might not get merged before the next v11 release, hence the reason for creating this PR 😄

Testing should be a simple case of checking the moved code and whether this still builds.

@bergmania
Copy link
Member

Looks good to me. and agree it is better as extension method

@bergmania bergmania merged commit 76c9977 into v11/dev Dec 13, 2022
@bergmania bergmania deleted the v11/feature/loggingsettingsextensions branch December 13, 2022 09:28
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