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

Disable Launchy and ignore executable command detection #75

Merged

Conversation

tricknotes
Copy link
Contributor

When Launchy runs dry run mode, its runner will detect executable command.
Ref: https://github.com/copiousfreetime/launchy/blob/v2.4.3/lib/launchy/detect/runner.rb#L58

It makes sense for Launchy. So letter_opener_web should be avoid this error.

This PR follows up #73.

@tricknotes tricknotes force-pushed the ignore-launcy-command-not-found branch 2 times, most recently from ef8dfc6 to 2a00b4b Compare January 16, 2018 11:27
@iguchi1124
Copy link
Contributor

@pseudomuto ping

Copy link
Contributor

@pseudomuto pseudomuto left a comment

Choose a reason for hiding this comment

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

Hey man, thanks for the PR! I have a small nitpick about a typo, but otherwise this LGTM 👍

@iguchi1124 thanks for pinging me

super
ENV['LAUNCHY_DRY_RUN'] = 'false'
rescue Launchy::CommandNotFoundError # rubocop:disable HandleExceptions
# Ignore for non-executable Launchy envirofment.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/envirofment/environment/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops :<
I fixed and rebased as 661ef92 .

When Launchy runs dry run mode, its runner will detect executable command.
Ref: https://github.com/copiousfreetime/launchy/blob/v2.4.3/lib/launchy/detect/runner.rb#L58

It makes sense for Launchy. So letter_opener_web should avoid this error.
@tricknotes tricknotes force-pushed the ignore-launcy-command-not-found branch from 2a00b4b to 661ef92 Compare January 28, 2018 23:57
@pseudomuto pseudomuto merged commit 30478d6 into fgrehm:master Jan 29, 2018
@tricknotes tricknotes deleted the ignore-launcy-command-not-found branch January 29, 2018 00:06
@tricknotes tricknotes restored the ignore-launcy-command-not-found branch January 29, 2018 00:08
@tricknotes
Copy link
Contributor Author

Thanks for your merge, @pseudomuto !

Could you release a new version?
I want to use this change as a released version. 😄

@pseudomuto
Copy link
Contributor

Just released v1.3.3

@tricknotes
Copy link
Contributor Author

Great thanks ✨

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.

3 participants