rails / thor

Thor is a toolkit for building powerful command-line interfaces.
http://whatisthor.com/
MIT License
5.12k stars 553 forks source link

Deprecation warning is confusing #728

Closed paddor closed 4 years ago

paddor commented 4 years ago

Thor says:

Deprecation warning: Thor exit with status 0 on errors. To keep this behavior, you must define exit_on_failure? in MyApp::Cli You can silence deprecations warning by setting the environment variable THOR_SILENCE_DEPRECATION.

I welcome the change to exit with a non-zero status on errors. So I won't define exit_on_failure?.

But what if I just want to silence this deprecation warning instead of all deprecation warnings from Thor ever?

deivid-rodriguez commented 4 years ago

I believe the way to silence this deprecation only and accept the new behaviour is to define exit_on_failure? to return true. Then once the old behaviour is completely removed, you can stop explicitly defining exit_on_failure? if you want.

If this is the trick, it could definitely be worded more clearly.

rafaelfranca commented 4 years ago

Like the message says, "you must define exit_on_failure?" in your class. You need to decide if you want thor to exit on failure, returning true or not exit on failure, returning false.

That explanation is on the CHANGELOG. Is the CHANGELOG not clear?

We can definitely improve the deprecation message, if you have a suggestion how to clarify, please send a PR.

deivid-rodriguez commented 4 years ago

The unclear bit in my opinion is "To keep this behavior, ". It should be clear that if you want the warning gone, you need to explicitly define exit_on_failure? regardless.

The explanation is the changelog is completely clear in this regard though.

paddor commented 4 years ago

Thanks. It's clear now.

Suggestion:

Deprecation warning: Thor used to exit with status 0 on errors. To silence this warning, define MyApp::Cli.exit_on_failure?.
You can silence all deprecation warnings by setting the environment variable THOR_SILENCE_DEPRECATION.