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

Drop command requires user confirmation or -y flag and does not allow arguments. #235

Merged
merged 7 commits into from
Sep 9, 2018

Conversation

Baggerone
Copy link
Contributor

This is in relation to issue #220.

@Baggerone Baggerone requested a review from a team as a code owner September 1, 2018 14:51
@Baggerone Baggerone mentioned this pull request Sep 2, 2018
@stanislas-m stanislas-m self-assigned this Sep 6, 2018
@stanislas-m stanislas-m merged commit 9e950f8 into gobuffalo:development Sep 9, 2018
@stanislas-m
Copy link
Member

Thanks!

@markbates
Copy link
Member

This is a REALLY big breaking change. A lot of people script around this, including this package. The ./test.sh script currently keeps prompting for user input and not running.

I think this needs to be reverted.

@stanislas-m
Copy link
Member

I'm against reverting the full change. I think we can at least keep the check for extra arguments: this will prevent behaviors such as the one exposed in the issue.

stanislas-m added a commit that referenced this pull request Sep 18, 2018
Drop command shouldn't need a confirmation, but we should help
to prevent bad mistakes.
@Baggerone
Copy link
Contributor Author

Baggerone commented Sep 18, 2018 via email

stanislas-m added a commit that referenced this pull request Sep 20, 2018
Drop command shouldn't need a confirmation, but we should help
to prevent bad mistakes.
mclark4386 pushed a commit that referenced this pull request Sep 28, 2018
… arguments. (#235)

* Don't allow args and get user confirmation through a flag or input.

* changed long form of confirm flag to "yes". Check for more than 0 args.

* Standardize confirmation hint.

* Add small fixes
mclark4386 pushed a commit that referenced this pull request Sep 28, 2018
Drop command shouldn't need a confirmation, but we should help
to prevent bad mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants