- Notifications
You must be signed in to change notification settings - Fork18
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
9596c44
to3a9cfc4
CompareUh oh!
There was an error while loading.Please reload this page.
3a9cfc4
tofa8cbf7
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Russtopia commentedJul 21, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
In general: how do we feel about the The downside is that An alternative could be to add a flag to
Where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Regarding the create/edit subcommands, I'd prefer to either keep how you have it right now in the pr, or go back to having justcreate
. Personally, I like that it's idempotent(ish) and I'd find it annoying to have to calledit
in the case of me forgetting that a devurl already exists. I like my CLIs to just do what I mean.
Uh oh!
There was an error while loading.Please reload this page.
ea7e23a
to3ced5b2
CompareUh oh!
There was an error while loading.Please reload this page.
3ced5b2
tob4109bf
Compare
Sure. Since@fuskovic added an alias ability to the cli recently, I left in "edit", but one can just use |
b4109bf
to5d92778
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please add integration tests to ensure continued compatibility.#81 should be a good model.
Uh oh!
There was an error while loading.Please reload this page.
7924bb3
tocc55a4a
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
@@ -137,35 +223,27 @@ func (sub delSubCmd) Run(fl *pflag.FlagSet) { | |||
exitUsage(fl) | |||
} | |||
if !portIsValid(port) { | |||
portNum, err := validatePort(port) | |||
if err != nil { | |||
exitUsage(fl) | |||
} |
RusstopiaJul 31, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What's your preference on exit/fatal from 'helper' funcs?
Current way here prints out usage and exits with nonzero status which is what we'd want, I guess. If I tucked in theif err != nil { exitUsage(f1) }
right into those two helpers it would simplify the flow here. However, to callexitUsage(fl)
within, I'd have to passfl
to both helpers which seems icky so perhaps I'll just leave these alone.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good to merge.
@Russtopia Anything blocking this from merging? |
Uh oh!
There was an error while loading.Please reload this page.
This implementshttps://github.com/cdr/enterprise/issues/4411
NOTE: this is intended for post 1.9 release. It involves a breaking API change to the devurl API and should be released in coordination withhttps://github.com/cdr/enterprise/pull/4381
What This Does
urls create
subcommand option--name <label | "">
urls ls
command and-o [human | json]
options following syntax of theusers ls
commandedit
urls subcommand which is just an alias forcreate
; just for usage stylePropose to makecreate
single-purpose (only create a new devurl) such thatedit
is the only way to modify an existing devurlChanges for compatibility withhttps://github.com/cdr/enterprise/pull/4381
port
argument changed to int typeTesting
Usage Examples