Skip to content

cli: add tilt alpha create cmd#4440

Merged
nicks merged 1 commit into
masterfrom
nicks/ch11866
Apr 16, 2021
Merged

cli: add tilt alpha create cmd#4440
nicks merged 1 commit into
masterfrom
nicks/ch11866

Conversation

@nicks

@nicks nicks commented Apr 15, 2021

Copy link
Copy Markdown
Contributor

Hello @milas,

Please review the following commits I made in branch nicks/ch11866:

7a19733 (2021-04-15 16:03:34 -0400)
cli: add tilt alpha create cmd

2380e96 (2021-04-15 14:49:53 -0400)
cli: add tilt alpha create filewatch

Code review reminders, by giving a LGTM you attest that:

  • Commits are adequately tested
  • Code is easy to understand and conforms to style guides
  • Incomplete code is marked with TODOs
  • Code is suitably instrumented with logging and metrics

@nicks
nicks requested a review from milas April 15, 2021 20:04
@shortcut-integration

Copy link
Copy Markdown

This pull request has been linked to Clubhouse Story #11866: add tilt alpha create.

@nicks
nicks changed the base branch from master to nicks/filewatch April 15, 2021 20:04
@nicks
nicks force-pushed the nicks/ch11866 branch 2 times, most recently from aa97114 to b26a70f Compare April 15, 2021 20:07

// A human-friendly CLI for creating cmds.
//
// The name is unfortunate.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😂

Comment thread internal/cli/create_cmd.go Outdated
"Working directory of the command. If not specified, uses the current working directory.")
cmd.Flags().StringArrayVarP(&c.env, "env", "e", nil,
"Set environment variables in the form NAME=VALUE.")
cmd.Flags().StringSliceVar(&c.filewatches, "file-watch", nil,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a total bikeshed and not important, but since the command + API convention is just filewatch (no hyphen), it might make sense to carry that here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i agree! changed

@nicks
nicks force-pushed the nicks/filewatch branch from 2380e96 to 8954c3a Compare April 16, 2021 16:59
Base automatically changed from nicks/filewatch to master April 16, 2021 17:45
@nicks
nicks merged commit a56bdeb into master Apr 16, 2021
@nicks
nicks deleted the nicks/ch11866 branch April 16, 2021 20:57
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