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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update thor #62

Merged
merged 3 commits into from
Jan 11, 2021
Merged

Update thor #62

merged 3 commits into from
Jan 11, 2021

Conversation

tomorrowkey
Copy link
Contributor

@tomorrowkey tomorrowkey commented Jan 7, 2021

Our project need thor to be version 1 or later to install by other dependencies.
This PR will be update the gem.

Update gem

Thor shows warning when command line exit with error.
Implement class method to resolve.

$ ./bin/candy_check foo
Could not find command "foo".
Deprecation warning: Thor exit with status 0 on errors. To keep this behavior, you must define `exit_on_failure?` in `CandyCheck::CLI::App`
You can silence deprecations warning by setting the environment variable THOR_SILENCE_DEPRECATION.

@jnbt Please look this 馃槈

@coveralls
Copy link

coveralls commented Jan 7, 2021

Coverage Status

Coverage increased (+0.004%) to 98.643% when pulling 196073e on tomorrowkey:update-thor into 141a90c on jnbt:master.

@jnbt
Copy link
Owner

jnbt commented Jan 7, 2021

@tomorrowkey There is already another PR #59 which removes the explicit version dependency completely which I don't really like.

I have at least one project stuck at thor < 1.0, but your changes seems to be fully backwards compatible.

So maybe we could use a slightly more relaxes version requirement, e.g. < 2.0?

@tomorrowkey
Copy link
Contributor Author

So maybe we could use a slightly more relaxes version requirement, e.g. < 2.0?

OK. It is better for us.
I'll change the version specification right away.

@tomorrowkey
Copy link
Contributor Author

I changed version specification by git amend.
And added a test for .exit_on_failure? to keep testing coverage at 196073e

@tomorrowkey
Copy link
Contributor Author

@jnbt Please review again

@jnbt jnbt merged commit 1dd41f2 into jnbt:master Jan 11, 2021
@jnbt
Copy link
Owner

jnbt commented Jan 11, 2021

@tomorrowkey Thanks. Released as Version 0.4.0

@jnbt jnbt mentioned this pull request Jan 11, 2021
@tomorrowkey tomorrowkey deleted the update-thor branch January 12, 2021 07:39
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.

None yet

3 participants