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

Fix LoadError catching when it's not related to CommandsGenerator #8831

Merged
merged 2 commits into from
Apr 11, 2017

Conversation

KrauseFx
Copy link
Member

@KrauseFx KrauseFx commented Apr 8, 2017

We rescue LoadError so that we tell users to run the tool directly if CommandsGenerator is not available. However since we call .start inside that begin rescue block we'd catch all LoadErrors from within fastlane, and then show the confusing error message of

pilot can't be called via `fastlane pilot`, run 'pilot' directly instead

We rescue `LoadError` so that we tell users to run the tool directly if CommandsGenerator is not available. However since we call `.start` inside that `begin` `rescue` block we'd catch all LoadErrors from within fastlane, and then show the confusing error message of

```
pilot can't be called via `fastlane pilot`, run 'pilot' directly instead
```
Copy link
Collaborator

@taquitos taquitos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny name nit, but 👍

@@ -48,13 +48,14 @@ def take_off
require File.join(tool_name, "commands_generator")

# Call the tool's CommandsGenerator class and let it do its thing
Object.const_get(tool_name.fastlane_module)::CommandsGenerator.start
commands_generator_ref = Object.const_get(tool_name.fastlane_module)::CommandsGenerator
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: adding "_ref" seems redundant

Copy link
Collaborator

@hjanuschka hjanuschka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@KrauseFx KrauseFx merged commit 89d4fb8 into master Apr 11, 2017
@KrauseFx KrauseFx deleted the fix-load-error-display branch April 11, 2017 03:50
@fastlane-bot
Copy link

Hey @KrauseFx 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

@fastlane-bot
Copy link

Congratulations! 🎉 This was released as part of fastlane 2.27.0 🚀

@fastlane fastlane locked and limited conversation to collaborators Jul 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants