Skip to content
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

Allow apps:destroy when not in a project #2270

Merged
merged 3 commits into from Jun 26, 2016

Conversation

@guillaumewuip
Copy link
Contributor

@guillaumewuip guillaumewuip commented Jun 26, 2016

Previously, when not in a git project with a dokku remote, dokku apps:destroy failed because of git remote remove dokku error status.

@michaelshobbs
Copy link
Member

@michaelshobbs michaelshobbs commented Jun 26, 2016

How does heroku behave in this situation?

This client is intended to mimic the official Heroku behaviors and so we should definitely do what they do here.

@guillaumewuip
Copy link
Contributor Author

@guillaumewuip guillaumewuip commented Jun 26, 2016

heroku seems to give us two commands :

$ heroku help apps
    ...
    apps:delete                    #  permanently destroy an app
    apps:destroy --app APP         #  permanently destroy an app
    ...

I've just tested heroku apps:destroy --app <app> outside any project and it works :

capture d ecran 2016-06-26 a 17 50 14

@michaelshobbs
Copy link
Member

@michaelshobbs michaelshobbs commented Jun 26, 2016

Does it end up leaving the remote in your git repo?

@guillaumewuip
Copy link
Contributor Author

@guillaumewuip guillaumewuip commented Jun 26, 2016

In case I use heroku apps:delete in the projet repo, no :
capture d ecran 2016-06-26 a 18 04 57

@michaelshobbs
Copy link
Member

@michaelshobbs michaelshobbs commented Jun 26, 2016

Ok I expect that behavior. Does it leave the remote in the repo if you delete the app from another directory?

@guillaumewuip
Copy link
Contributor Author

@guillaumewuip guillaumewuip commented Jun 26, 2016

Yes the heroku remote is not touched when destroying the app from another directory :
capture d ecran 2016-06-26 a 18 10 45

@michaelshobbs
Copy link
Member

@michaelshobbs michaelshobbs commented Jun 26, 2016

Excellent! Let me get these tests running again and we'll get this merged.

Thanks for contributing!

@guillaumewuip
Copy link
Contributor Author

@guillaumewuip guillaumewuip commented Jun 26, 2016

@michaelshobbs thanks !

@michaelshobbs
Copy link
Member

@michaelshobbs michaelshobbs commented Jun 26, 2016

Looks like there is a bug here. Though I'm not 100% certain why based on the diff. If you have time to debug that would be great otherwise I can look at it later.

@@ -81,7 +89,7 @@ if [[ ! -z $DOKKU_HOST ]]; then
fi
;;
apps:destroy)
git remote remove dokku
is_git_repo && has_dokku_remote && remote remove dokku

This comment has been minimized.

@josegonzalez

josegonzalez Jun 26, 2016
Member

This is missing the git part of the remote remove dokku call.

`git remote show <remote>` seems to really check if the remote is a git repo
whereas we just want to know if a remote named "dokku" is present
@guillaumewuip guillaumewuip reopened this Jun 26, 2016
@michaelshobbs michaelshobbs merged commit 80d1323 into dokku:master Jun 26, 2016
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.