-
Notifications
You must be signed in to change notification settings - Fork 224
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
feat(spaces): migrate drains:{set,get} to spaces topic #2433
base: main
Are you sure you want to change the base?
Conversation
This commit partially migrates the `drains:set` and `drains:get` commands for `spaces` under the `spaces` topic, so they can be accessed from `spaces:drains:set` and `spaces:drains:get`. The rationale here is that these are `spaces`-specific commands, and there is significant semantic overlap between `drains:add` and `drains:set`. This makes it clearer that these commands are `spaces`-specific. In order to not break comaptibility with existing workflows, the old `drains:{set,get}` commands will continue to work, but remain hidden.
3b959d2
to
57d306a
Compare
Drains can be set for Common Runtime apps, so we can't hide the old command. Very weird to have to use spaces:drains to set for a CR app. Even if we did this, our doc pages would have to change as well. |
So, I could imagine there being two places to set log drains because one is specific to an app ( Not sure that is an improvement over the current Lets hold off on this until we can get a more complete perspective on what this means for DX. |
@chrismarino I think you might have fallen into the trap that this PR looks to mitigate. This change (
Both require a Then the non-spaces-specific:
The purpose of this PR is to make it very clear that FWIW, |
This commit partially migrates the
drains:set
anddrains:get
commands forspaces
under thespaces
topic, so they can be accessed fromspaces:drains:set
andspaces:drains:get
.The rationale here is that these are
spaces
-specific commands, and there is significant semantic overlap betweendrains:add
anddrains:set
. This makes it clearer that these commands arespaces
-specific.In order to not break comaptibility with existing workflows, the old
drains:{set,get}
commands will continue to work, but remain hidden.