-
-
Notifications
You must be signed in to change notification settings - Fork 235
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 to .NET 8 and leverage keyed services for decoration #209
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #209 +/- ##
==========================================
+ Coverage 64.71% 66.15% +1.44%
==========================================
Files 25 24 -1
Lines 1278 1232 -46
Branches 0 112 +112
==========================================
- Hits 827 815 -12
+ Misses 451 387 -64
- Partials 0 30 +30 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. A lot nicer than the DecoratedType file, it is nasty. Surprised by how little of the code actually changes. This should allow more compatibility.
Thanks for the review, @DanHarltey! All good feedback 👍🏻 |
Hi @khellang Please, please, please :) PS |
I can probably target RC2 and publish a prerelease version if anyone's eager to test it out. |
@khellang Yes please 🙏🏻 |
Hi @khellang 😃 Now I can wait for released version of net8 and newest scrutor 😃 |
What does that mean? How do you envision using keyed services with Scrutor? |
I would love to register services from given assembly as keyed with specified key, For example. I have modular monolith. Each module is a separated project/assembly. I execute For now I am using this hacky solution 😄 var tempServices = new ServiceCollection();
tempServices.Scan( );
foreach (var service in tempServices)
{
// register service as keyed
} It would be great to do this something like: new ServiceCollection()
.Scan(s => s.FromAssembliesOf(module.GetType())
.AddClasses(c =>
{
c.AssignableToAny(
typeof(ICommandHandler<,>),
typeof(ICommandHandler<>),
typeof(IQueryHandler<,>),
typeof(IEventHandler<>),
typeof(IDomainEventHandler<>))
.WithoutAttribute<DecoratorAttribute>();
}, false)
.AsKeyed(moduleName) // <========
.AsImplementedInterfaces()
.WithScopedLifetime()); |
If it's allowed to register multiple services against the same key, it could work. I'll have to verify it. |
Hi @khellang :) |
Hi @khellang ? How about now, after NET8 premiere? 😄 |
Still waiting :) |
@rekosko What exactly are you waiting for? The latest version on NuGet works fine on .NET 8, right? |
@khellang yes I confirm it works on .NET 8 :) But decoration for keyed services is not yet implemented or I missed the purpose of this PR? |
That's not the purpose of this PR, no. Although I want to add that as well before shipping a new version. This PR replaces an old hack to avoid StackOverflowExceptions when decorating services with keyed services under the covers. It also files a few other long-standing issues and pending breaking changes. |
BTW, if anyone wants to contribute support for decorating keyed services to get a release out sooner, that would be cool. |
@khellang Can this be merged into master or it's not complete or it needs more testing? Or it's better to base all the tests and work on |
It's not totally complete and needs more tests. It's better to base work off this branch for now. Once this goes into master, I will release a new major version. |
Gentle ping on this. I wonder what's the plan/fate for this PR? |
Howdy! Will Scrutor be ported to .NET 8? |
Just because the library targets .NET 6, doesn't mean it doesn't support .NET 8. It's perfectly fine to use packages targeting older framework versions on newer frameworks. Have you encountered any issues using the package on .NET 8? |
No, just curious, we are considering using Scrutor in one of our projects, but stricter corporate policies prevent us from using packages targetting .NET 6 🤷 |
It's obvious that someone in your company doesn't have a clue how .NET works - unfortunately I can't help with that 😅 What version a package targets it 100% irrelevant to support policies - it's the version of your runtime that matters. If you're running .NET 8 using a package that targets anything newer than .NET Core 2, you're totally supported. I'm curious; how do you enforce that? 🤔 I can't really commit to anything, as I have a lot going on in my life right now and the package works fine on .NET 8 as-is. I'll do my best to get this PR in and ship .NET 8 support when I can find the time. |
Understandable; life is a priority, as it ought to be. I have not contributed to this project before, but would like to know if there is any way for someone to possibly assist so you don't have to do what life won't permit you to have time for. I don't know what I'll have time for either, but if there's a way to possibly help get this over the line, I'd like to help. |
What do you think @DanHarltey?
Closes #171
Closes #208
Closes #65
Closes #200