Skip to content

Abort early if generator command fails#34420

Merged
y-yagi merged 7 commits intorails:masterfrom
deivid-rodriguez:abort_early_if_generator_command_fails
Dec 7, 2018
Merged

Abort early if generator command fails#34420
y-yagi merged 7 commits intorails:masterfrom
deivid-rodriguez:abort_early_if_generator_command_fails

Conversation

@deivid-rodriguez
Copy link
Contributor

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

Summary

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!

@rails-bot
Copy link

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 Compare November 16, 2018 11:29
@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 Compare December 7, 2018 03:25
@y-yagi y-yagi merged commit f173ec7 into rails:master Dec 7, 2018
@y-yagi
Copy link
Member

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants