Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

fix: Optional --name for urls create [ch8011] #340

Merged
merged 2 commits into from
May 10, 2021
Merged

fix: Optional --name for urls create [ch8011] #340

merged 2 commits into from
May 10, 2021

Conversation

greyscaled
Copy link
Contributor

@greyscaled greyscaled commented May 10, 2021 •
edited
Loading

Demo

Help

$ go run ./cmd/coder/main.go  urls create --help
Create a new dev URL for a workspace

Usage:
  coder urls create [workspace_name] [port] [flags]

Aliases:
  create, edit

Examples:
coder urls create my-workspace 8080 --name my-dev-url

Flags:
      --access string   Set DevURL access to [private | org | authed | public] (default "private")
  -h, --help            help for create
      --name string     DevURL name
      --scheme string   Server scheme (http|https) (default "http")

Global Flags:
  -v, --verbose   show verbose output

Create

$ go run ./cmd/coder/main.go urls create grey-m-built-in 3001
success: created devurl for port 3001

List

URL                                                  Port    Access     Name     Scheme    
https://3001-grey-m-built-in-grey.<REST_IS_OBFUSCATED>/    3001    PRIVATE             http

@shortcut-integration
Copy link

This pull request has been linked to Clubhouse Story #8011: The coder-cli requires naming dev URLs, but the dashboard does not.

@greyscaled greyscaled requested review from bpmct and removed request for bpmct May 10, 2021 21:08
@greyscaled greyscaled self-assigned this May 10, 2021
@greyscaled greyscaled requested a review from jawnsy May 10, 2021 21:11
@jawnsy jawnsy requested review from f0ssel and removed request for jawnsy May 10, 2021 21:12
jawnsy
jawnsy reviewed May 10, 2021
View reviewed changes
Copy link
Contributor

@jawnsy jawnsy left a comment

Choose a reason for hiding this comment

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

LGTM

f0ssel
f0ssel approved these changes May 10, 2021
View reviewed changes
internal/cmd/urls.go
@@ -195,8 +195,6 @@ func createDevURLCmd() *cobra.Command {
cmd.Flags().StringVar(&access, "access", "private", "Set DevURL access to [private | org | authed | public]")
cmd.Flags().StringVar(&urlname, "name", "", "DevURL name")
cmd.Flags().StringVar(&scheme, "scheme", "http", "Server scheme (http|https)")
_ = cmd.MarkFlagRequired("name")
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming this is tested to see if the empty string still being in the payload doesn't break anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, see PR description - you get the standard naming scheme that you get in the UI when no name is provided

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$ go run ./cmd/coder/main.go urls create grey-m-built-in 3002
warning: version mismatch detected
  | Coder CLI version: unknown
  | Coder API version: 1.18.0-rc.1+321-g7c364f3ab-20210510
  | 
  | tip: download the appropriate version here: https://github.com/cdr/coder-cli/releases
success: created devurl for port 3002

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot from 2021-05-10 16-57-10

@greyscaled greyscaled merged commit 08ef03e into master May 10, 2021
@greyscaled greyscaled deleted the vapurrmaid/ch8011/dev-url-cli branch May 10, 2021 21:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
2 more reviewers

@jawnsy jawnsy jawnsy left review comments

@f0ssel f0ssel f0ssel approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@greyscaled greyscaled

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@greyscaled @jawnsy @f0ssel