hk destroy with app name needs a git remote?

kr / hk

Fast Heroku client

https://hk.heroku.com/

77 stars 6 forks source link

hk destroy with app name needs a git remote? #83

Closed bgentry closed 10 years ago

bgentry commented 10 years ago

$ hk destroy bgdnstest
could not find git remote heroku in /Users/bgentry

$ hk help destroy
Usage: hk destroy app

Destroy destroys a heroku app.

There is no going back, so be sure you mean it.

Example:

    $ hk destroy myapp

Maybe this got broken when @tt made changes to the -a flag?

Do we want to keep the hk destroy appname syntax, or go with hk -a appname destroy ?

kr commented 10 years ago

The hk destroy appname syntax exists for two reasons:

symmetry with hk create appname
require the user to provide the name for safety. If you allow the -a form, which implies using the git remote by default, then you have to add a --confirm option or something equally awful. This nicely avoids that problem.

bgentry commented 10 years ago

@kr Sure, that makes sense. So it's just a bug that it doesn't work as documented right now.

kr commented 10 years ago

Yes, that sounds right.

tt commented 10 years ago

It's actually a good thing that my change made this more visible; before the bug would just prohibit you to destroy an app if the "heroku" remote was set to a different app with provisioned add-ons. I'm sorry for not testing this, though.

Fixed in #84.

tt commented 10 years ago

An idea regarding user experience would be to prompt about destroying the resolved app if you didn't give any arguments. Especially with auto-generated app names, this would be a lot more convenient.

$ hk destroy
Destroy "foo" [y/N]: ...

The same could go for hk init. If you're in an empty directory (or just one that doesn't specify a "heroku" remote), you could suggest the app name to be the directory name.

kr commented 10 years ago

Yeah, this suggestion has come up several times before in the old heroku CLI. Unfortunately, it doesn't work as well as one would hope. Users habituate to the prompt, and it loses its effectiveness.

http://alistapart.com/article/neveruseawarning

If we are going to look for better alternatives, an undo option would be most awesome. But that requires some large changes in the platform, and ripples out to add-on providers as well.

Meanwhile, requiring the user to explicitly provide the app name is simple and effective. For the interactive case, users have to type or copy and paste the name, providing positive affirmation that they're destroying the intended app. It also plays nicely with scripts and other automated tools.

bgentry commented 10 years ago

Fixed by #84, thanks @tt !