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

Set elm path #1

Merged
merged 3 commits into from
Aug 10, 2015
Merged

Set elm path #1

merged 3 commits into from
Aug 10, 2015

Conversation

urfolomeus
Copy link
Contributor

I wanted to use elm-brunch on a Phoenix project that has the elm file within a web/elm folder, but brunch-config.js on the root. However doing this was running elm make on the root of the project, thus installing elm-stuff and elm-package.json on the root. This in turn meant that the elm file was unable to find its dependencies.

This PR allows you to set a config.plugins.elmBrunch.elmFolder option (defaults to null), which is then passed into the exec command as the cwd option (setting this to null by default means that current behaviour is maintained).

I also wrote some tests around this to ensure I hadn't broken anything, but I put these on a separate commit in case you'd rather not include them or their dependencies.

@madsflensted
Copy link
Owner

Hi @urfolomeus thanks for the PR that is a good feature.
Also great that you added tests, but it looks like the second commit accidentally includes the node_modules folder with everything. Could I get you remove that folder then I will be happy to merge the PR.

@urfolomeus
Copy link
Contributor Author

Oops! Yeah good point. I'll add it the gitignore and force push when I get back home. :)

@urfolomeus
Copy link
Contributor Author

@madsflensted Done now. :)

Would you like me to squash the commits or are you happy with them as is?

madsflensted added a commit that referenced this pull request Aug 10, 2015
@madsflensted madsflensted merged commit 7b63f8b into madsflensted:master Aug 10, 2015
@madsflensted
Copy link
Owner

@urfolomeus thanks again!

@urfolomeus
Copy link
Contributor Author

@madsflensted thanks for accepting :)

@madsflensted
Copy link
Owner

published new version to npm

@urfolomeus
Copy link
Contributor Author

👍

@urfolomeus urfolomeus deleted the set-elm-path branch August 11, 2015 09:04
@ibnHatab ibnHatab mentioned this pull request Oct 12, 2015
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.

2 participants