cli icon indicating copy to clipboard operation
cli copied to clipboard

feat(spaces): migrate drains:{set,get} to spaces topic

Open mble-sfdc opened this issue 2 years ago • 3 comments

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.

mble-sfdc avatar Aug 11 '23 10:08 mble-sfdc

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.

chrismarino avatar Aug 17 '23 20:08 chrismarino

So, I could imagine there being two places to set log drains because one is specific to an app (drains:set) , while the other is set for the entire space (space:drain:set) as suggested here.

Not sure that is an improvement over the current drains:set --space

Lets hold off on this until we can get a more complete perspective on what this means for DX.

chrismarino avatar Aug 17 '23 21:08 chrismarino

@chrismarino I think you might have fallen into the trap that this PR looks to mitigate.

This change (spaces:drains:set, spaces:drains:get) only acts on the space-specific semantic (drains:set, drains:get). For non-spaces apps, that uses drains:add. Here is the help output of the spaces-specific commands:

$ heroku help drains:get
display the log drain for a space

USAGE
  $ heroku drains:get -s <value> [--json]

FLAGS
  -s, --space=<value>  (required) space for which to get log drain
  --json               output in json format

DESCRIPTION
  display the log drain for a space
$ heroku help drains:set
replaces the log drain for a space

USAGE
  $ heroku drains:set URL -s <value>

FLAGS
  -s, --space=<value>  (required) space for which to set log drain

DESCRIPTION
  replaces the log drain for a space

Both require a --space flag.

Then the non-spaces-specific:

$ heroku help drains
display the log drains of an app

USAGE
  $ heroku drains -a <value> [--json] [-r <value>]

FLAGS
  -a, --app=<value>     (required) app to run command against
  -r, --remote=<value>  git remote of app to use
  --json                output in json format

DESCRIPTION
  display the log drains of an app


COMMANDS
  drains:add     adds a log drain to an app
  drains:remove  removes a log drain from an app
$ heroku help drains:add
adds a log drain to an app

USAGE
  $ heroku drains:add URL -a <value> [-r <value>]

FLAGS
  -a, --app=<value>     (required) app to run command against
  -r, --remote=<value>  git remote of app to use

DESCRIPTION
  adds a log drain to an app

The purpose of this PR is to make it very clear that drains:set and drains:get only apply within a spaces context, hence moving them under the spaces topic.

FWIW, drains:get and drains:set are already hidden commands - this PR doesn't change that, only adding the alias in the spaces topic (e.g. spaces:drains:set).

mble-sfdc avatar Aug 17 '23 21:08 mble-sfdc