-
Notifications
You must be signed in to change notification settings - Fork 7
add installer #769
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
add installer #769
Conversation
|
Hi , Thank you for your contribution. We have routed the PR to the appropriate team and they will get back to you as soon as possible. Kind regards, |
|
Kind reminder. |
|
The request is still with the relevant team for review and implementation. I appreciate your patience! |
|
@hasancanguler Thanks for the contribution. I'm currently reviewing your PR and planning to pull things into the codebase while potentially undoing the renames to keep the feature names as is. Generally we are also trying to keep the public API surface as lean as possible, so I would also go ahead and internalize the installers. Is there a particular reason you made those types public in your PR? Do you need that functionality to be invokable outside NServiceBus for some other tooling purposes? Additionally, our persistences usually provide a way to decouple disabling the creation of the infrastructure from the installers. We have seen some customers having valid reasons to have installers enabled while disabling the index creation. That's the reason why all persisters have some kind of https://docs.particular.net/persistence/sql/install#script-execution-in-non-development-environments Another question. Are you currently in need of this in your project, or can this wait to be released in the next major release (quite likely to be released around the release of .NET 10)? |
|
We have pulled the changes into #791 |
|
hi @danielmarbach, thank you for your questions and review. There is no specific reason to keep it public; we can standardise the access modifiers. |
This installer has been added to solve the following issue:
#55