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

Allow for a local installation of aglio #51

Merged
merged 2 commits into from
Mar 31, 2019

Conversation

alappe
Copy link
Contributor

@alappe alappe commented Mar 20, 2019

To use it locally:

npm install aglio in your project using blue_bird, then set aglio_path: "./node_modules/.bin/aglio in the config.

Application.get_env(:blue_bird, :aglio_path, "aglio")
|> Path.absname()

if File.exists?(aglio_path) == false do
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR! Unfortunately, this doesn't work like it is, because the default value aglio is not a complete path to the binary and File.exists?/1 doesn't find it (see ci). Perhaps you could make a check on the :aglio_path first: If a value is set, use File.exists?/1, and if it is not set, try to find the globally installed executable with System.find_executable/1.

What do you think?

@rhazdon

Copy link
Member

Choose a reason for hiding this comment

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

Oh, and if you implement it that way, it would be great to have the error message reflect both ways.

@alappe
Copy link
Contributor Author

alappe commented Mar 20, 2019

Sure, I'll adapt it tomorrow. I was on the way to a meeting this morning and this was the minimal case I needed to make it work on my machine. So I thought I'd share for comments before I forget it at all ;-)

@woylie woylie merged commit 9bed25b into KittyHeaven:master Mar 31, 2019
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

2 participants