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

implement run #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

implement run #19

wants to merge 1 commit into from

Conversation

tobi
Copy link
Contributor

@tobi tobi commented Jul 1, 2024

Made a reasonable implementation of dev run command

vscode ➜ /workspaces/u2 (main) $ dev run server
Could not find scenic-mysql_adapter-1.0.1, mysql2-0.5.5 in locally installed gems
Run `bundle install` to install missing gems.
vscode ➜ /workspaces/u2 (main) $ dev run

Available commands:

  
  console
  server
  services
  test
  test-gsd
  foreman
  docs
  logs
    logs:server
    logs:sidekiq
    logs:scheduler-clock
    logs:environment
    logs:storybook
    logs:docs
  style
  staging
  kafka-consumer
  kafka-seed
    kafka-seed:github-events
vscode ➜ /workspaces/u2 (main) $ dev run logs

Available commands:

  logs
    logs:server
    logs:sidekiq
    logs:scheduler-clock
    logs:environment
    logs:storybook
    logs:docs

@burke
Copy link
Owner

burke commented Jul 2, 2024

Ok. I see what you're getting at here. Here's what I think we should do:

  1. Possibly change the behaviour upstream of dev run as described below
  2. Change the project-local task runner to be more natively subcommand-aware

on dev run

Currently dev run selects the "most useful" command by some heuristic. Most commonly this means either dev server or dev test. It does feel like the "run" command should probably be able to run the custom commands like other tools; the difference with dev is that we lifted these into the toplevel namespace.

This gets into some really fundamental UX design concerns. We've always treated the "big 5" project-local commands (server, console, build, test, style) as slightly special.

One possible design here would be for just those five to remain at the toplevel and everything else project-local be accessed through a dev run, freeing us from global/project command name conflicts.

Another option would be for dev run to basically be a redundant insertion of the word run into the command, except for not presenting or interacting with any of the dev-global commands (e.g. up, clone, cd, and so on). We would, in this case, scope the help and command lookups to only the project-local defined commands.

I don't like either of these schemes on balance: the former requires more typing and creates a divide between blessed and custom commands; the latter creates confusion through things like "wait, is dev lint the same as dev run lint?".

Open to ideas there. In either case, it does make sense to deal with subcommands better...

on subcommands

Subcommands have always been really hacky and half-supported in cli-kit. In the most general case I think it doesn't make sense for a tool to present all its subcommands up front in a tree view, but I think in the case of a dev project it feels appropriate for reasons I can't put my finger on precisely.

One thing, though, is that currently commands can have subcommands but also be directly invokable... for example, dev logs can have behaviour, but also support subcommands` (and these are handled with a space, not a colon, which I like better but I don't have strong feelings).

Anyway, we do have that data available as a tree and we should present it on dev help, whatever the specifics.

@tobi
Copy link
Contributor Author

tobi commented Jul 9, 2024

Outside of the way it presents the full tree (which I like) it does match the current dev run behavior unless I'm very mistaken here.

This patch is meant to give the same run semantics to mini dev that the normal dev tool has, and deals with all the weird leaf notes configurations that might exist. It doesn't implement the heuristic part, but this is also terrible and we should get rid of it imo.

@burke
Copy link
Owner

burke commented Aug 16, 2024

Run actually doesn't work the way you'd think it does, but we're changing that in real dev soon, then we'll make this match.

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