-
Notifications
You must be signed in to change notification settings - Fork 259
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
Move scopes inside #3454
Move scopes inside #3454
Conversation
As scopes are now done in the Handler Factories Command Processor Providers are no longer needed
@iancooper let me know if you're happy with this direction |
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.
✅ Code Health Quality Gates: OK
Change in average Code Health of affected files: +0.01 (9.52 -> 9.53)
- Improving Code Health: 3 findings(s) ✅
- Affected Hotspots: 2 files(s) 🔥
src/Paramore.Brighter.ServiceActivator/ControlBus/ControlBusReceiverBuilder.cs
Show resolved
Hide resolved
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.
✅ Code Health Quality Gates: OK
Change in average Code Health of affected files: +0.01 (9.52 -> 9.53)
- Improving Code Health: 3 findings(s) ✅
- Affected Hotspots: 2 files(s) 🔥
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.
✅ Code Health Quality Gates: OK
Change in average Code Health of affected files: +0.01 (9.52 -> 9.53)
- Improving Code Health: 3 findings(s) ✅
- Affected Hotspots: 2 files(s) 🔥
When I publish, particularly in parallel, I would expect state to be fresh. I think the problem with reusing scope, is that's not true. I think we need to ask for a fresh scope for each publish chain. I like where we are, so hopefully we can do that through the lifetime somehow. |
The only other alternative is to make them transient within the scope i.e. the publish has the scope, the publish chains are transient within that scope. |
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.
❌ Code Health Quality Gates: FAILED
Change in average Code Health of affected files: -0.01 (9.52 -> 9.51)
- Declining Code Health: 1 findings(s) 🚩
- Improving Code Health: 3 findings(s) ✅
- Affected Hotspots: 2 files(s) 🔥
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.
❌ Code Health Quality Gates: FAILED
Change in average Code Health of affected files: -0.01 (9.52 -> 9.51)
- Declining Code Health: 1 findings(s) 🚩
- Improving Code Health: 3 findings(s) ✅
- Affected Hotspots: 2 files(s) 🔥
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.
❌ Code Health Quality Gates: FAILED
Change in average Code Health of affected files: -0.01 (9.50 -> 9.50)
- Declining Code Health: 1 findings(s) 🚩
- Improving Code Health: 3 findings(s) ✅
- Affected Hotspots: 2 files(s) 🔥
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.
I think this a solid approach and follows what we would expect the scope to be.
At the minute the Factory Scope (Dependency injection) is around the command processor, this means that when Publish is called multiple Handlers could share the same scope which would be unexpected by the users.
#3356