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

Abort early if generator command fails #34420



Copy link

@deivid-rodriguez deivid-rodriguez commented Nov 10, 2018


This PR happens to overlap with #34418, but I think it tries to fix a slightly different problem. My issue is that sometimes rails generator commands that I run in CI fail, and I don't notice because the generator does not abort. So I added a feature in thor to force aborting the process if this happens, which I'm using in this PR. I think this could maybe become the default, but for now I introduced it as an opt-in because there's situations where you don't want it (for example, if you do rails db:drop inside a generator, you probably want to ignore the case where the db does not exist).

In my case, I run into this problem when using the generate action, so I refactored it to reuse the rails_command action, so I only need to add the new abort_on_failure option in a single place.

This probably needs a change log entry and some docs but I wanted to gathers some opinions first!

Copy link

@rails-bot rails-bot commented Nov 10, 2018

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

@deivid-rodriguez deivid-rodriguez force-pushed the abort_early_if_generator_command_fails branch from b4ee86d to cc7a5d1 Nov 16, 2018
@rails-bot rails-bot bot added the docs label Nov 16, 2018
@deivid-rodriguez deivid-rodriguez force-pushed the abort_early_if_generator_command_fails branch from f3ea7ff to f03f935 Dec 7, 2018
@y-yagi y-yagi merged commit f173ec7 into rails:master Dec 7, 2018
2 checks passed
Copy link

@y-yagi y-yagi commented Dec 7, 2018

@deivid-rodriguez Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants