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

update CommonServiceLocator to remove Microsoft.NETCore.App dependancy #302

Merged
merged 4 commits into from
Jul 26, 2024

Conversation

hahn-kev
Copy link
Contributor

@hahn-kev hahn-kev commented Jul 16, 2024

closes sillsdev/languageforge-lexbox#964

LCM references CommonServiceLocator, which in turn references Microsoft.NETCore.App

this breaks various applications when attempting to include LCM. This PR updates that dependancy.

I also did a bit of refactoring to remove references to IServiceLocator in favor of the BCL interface IServiceProvider. I then provided some extension methods so there is source compatibility with this new version. If this is too big of a change and impacts FieldWorks too much then I can remove it and just upgrade the package. However the namespace that IServiceLocator is in has changed as part of the version change, so this seemed like a good time to remove our usage of this interface as it's non standard.


This change is Reviewable

…erviceLocator with IServiceProvider as this is now in the BCL. Write helper class to provide source compatability with IServiceLocator.
Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

I need to build locally and verify this with FieldWorks, but it looks promising.

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @hahn-kev)


tests/SIL.LCModel.Tests/DomainImpl/NotebookTests.cs line 5 at r1 (raw file):

// (http://www.gnu.org/licenses/lgpl-2.1.html)

using System;

huh?

@hahn-kev
Copy link
Contributor Author

tests/SIL.LCModel.Tests/DomainImpl/NotebookTests.cs line 5 at r1 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

huh?

IServiceProvider is declared in the system namespace.

@jasonleenaylor
Copy link
Contributor

@hahn-kev here is an issue for FieldWorks we still need to work out
fw\Src\LexText\LexTextControls\LiftMergerSupportCodeAndClasses.cs(4227,38): error CS1061: 'ILcmServiceLocator' does not contain a definition for 'GetAllInstances' and no accessible extension method 'GetAllInstances' accepting a first argument of type 'ILcmServiceLocator' could be found (are you missing a using directive or an assembly reference?) [C:\Repositories\fwroot\fw\Src\LexText\LexTextControls\LexTextControls.csproj]

@hahn-kev
Copy link
Contributor Author

@jasonleenaylor I added in a drop in, however the IServiceProvider interface is more narrow and there's just a single code path for fetching services and you're expected to just request an IEnumerable<T> where as the Structure map IoC container has very different code paths so I couldn't do the same thing and put a special case when it's our structure map container.

@jasonleenaylor
Copy link
Contributor

Next in line:
fw\Src\LCMBrowser\LCMBrowserForm.Designer.cs(30,29): error CS1061: 'ILcmServiceLocator' does not contain a definition for 'GetInstance' and no accessible extension method 'GetInstance' accepting a first argument of type 'ILcmServiceLocator' could be found (are you missing a using directive or an assembly reference?) [C:\Repositories\fwroot\fw\Src\LCMBrowser\LCMBrowser.csproj]

@hahn-kev
Copy link
Contributor Author

Next in line: fw\Src\LCMBrowser\LCMBrowserForm.Designer.cs(30,29): error CS1061: 'ILcmServiceLocator' does not contain a definition for 'GetInstance' and no accessible extension method 'GetInstance' accepting a first argument of type 'ILcmServiceLocator' could be found (are you missing a using directive or an assembly reference?) [C:\Repositories\fwroot\fw\Src\LCMBrowser\LCMBrowser.csproj]

I think that one will just need a using statement. Though we could also just add the methods to the ILcmServiceLocator interface instead of using Extension methods. Not sure the best path forward there.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

With minor changes I got FieldWorks to build with all tests passing. A smoke test showed things apparently working.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @hahn-kev)

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @hahn-kev)

@hahn-kev hahn-kev enabled auto-merge (squash) July 26, 2024 03:25
@hahn-kev hahn-kev merged commit e68ba41 into master Jul 26, 2024
3 checks passed
@hahn-kev hahn-kev deleted the lexbox-964 branch July 26, 2024 03:29
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.

maui desktop does not start due to Microsoft.NETCore.App dependancy coming from LCM
2 participants