-
Notifications
You must be signed in to change notification settings - Fork 396
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
[Idea] [Breaking changes!] Add shorthand flags #171
Conversation
Nice, I like it! @aybabtme what do you think? |
AddStringFlag(cmdDomainCreate, doctl.ArgIPAddress, "", "IP address", requiredOpt()) | ||
|
||
AddStringFlag(cmdDomainCreate, doctl.ArgIPAddress, "", "", "IP address", requiredOpt()) | ||
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.
Tiny nitpick, extra whitespace.
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.
@nanzhong fixed. :D I hope it's correct now. Also fixed indentation for volumes.go
at one point.
And yea, commit should be formatting but whatever. :P
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.
lgtm!
package doctl | ||
|
||
const ( | ||
/** |
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.
I'm curious why this large block of commented out code?
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.
My assumption was some placeholder in case we want to introduce more short flags for other things?
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.
Thanks @xmudrii for all your contributions. We realize that we've been a little slow to respond, but hoping to change that in the coming year. :) 🎆 |
No problem, glad to help. :) And that's not a problem. :) |
fixed imports added copyright headers vendoring godo update changes for digitalocean#171
fixed imports added copyright headers vendoring godo update changes for digitalocean#171
I'm starting this as idea to make shorthand flags work.
Current status when you execute
doctl compute droplet delete
:If you want to force delete, you'll have to type
--force
. I don't see that natural in Unix world, as it's mostly normal to have shorthand flag (-f
).When I was implementing delete confirm in #148, there was no way to do it, and at that point I had no time to play around it.
That's not only for force, it's for all flags.
This is ready code, it's fully usable (I'm not sure about tests tho), and it's working.
I added one more argument for shorthand
Add*Flag
and changed from*
to*P
(e.g.String
toStringP
).I feel like it could be done better, but let's discuss and give me idea if you agree with adding shorthand flags.
When you execute same command after chagnes:
Both
-f
and--force
are working!I hope you like idea.
cc @aybabtme @phillbaker @andrewsomething @nanzhong @mchitten