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

Fix DI scope disposal and support for overridding the default command #95

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

tgardner
Copy link
Contributor

refactor(DependencyInjectionCommandCreator): use ActivatorUtilities for command creation

refactor(HostedCommandExtensions): simplify Oakton registration and command execution

feat(HostedCommandExtensions): add OaktonOptions for configuration

refactor(HostedCommandExtensions): remove HostedCommandOptions in favor of OaktonOptions

test(HostedCommandsTester): update tests to use OaktonOptions and default command

…or command creation

refactor(HostedCommandExtensions): simplify Oakton registration and command execution

feat(HostedCommandExtensions): add OaktonOptions for configuration

refactor(HostedCommandExtensions): remove HostedCommandOptions in favor of OaktonOptions

test(HostedCommandsTester): update tests to use OaktonOptions and default command
@tgardner
Copy link
Contributor Author

@jeremydmiller This should address your concerns with the scope disposal mentioned in #93 (comment)

Also added the ability to override the default command during the configuration stage, based on a discord conversation

@jeremydmiller jeremydmiller merged commit a70a0f8 into JasperFx:master Sep 23, 2024
1 check passed
@jeremydmiller
Copy link
Member

@tgardner I got this one in, but one more really important thing (I took care of it), you needed to ensure that the IHost was disposed too so that it isn't hanging on to locks or resources or who knows what. Thanks for doing this!

@tgardner tgardner deleted the fix-scope-disposal branch September 23, 2024 22:17
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