-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Rollback npm scripts #1322
Comments
Done! |
Guys, do you happen to remember what was the issue with those two PRs? I'd love to take another look at it and fix it. |
@mareksuscak we still ship with the scripts but we don't use them by default in your config/dev.exs because they were not working on some Windows machines. |
@josevalim Yup I know. I actually read your comment but there are no further details which I'm interested in. I could possibly help you guys debug and fix the issue so that we can have a truly easy to change asset pipeline. |
Any chance it's been this eaccess error related to the lack of shebang support on Windows? EDIT: Adding a reference to this comment for my own convenience. |
No idea. If someone would let to debug widows support for npm scripts we'd accept a PR that was sure to work. As is today, running without npm scripts in package.json works across all platforms so it's not a huge priority for us to track this issue down. |
To add to that, the flip to and from npm scripts is literally a one line of code change. So if someone can track it down and guarantee it works on Windows, that's great, but indeed low priority on our side. |
Sure, I understand. I have extensive experience with Node and JavaScript and do have access to a dev Windows machine. I am willing to put in some time to debug the issue and contribute to Phoenix as I think it's a great project (and so is Elixir!). I just need some clue to start with. I'll start with this eaccess error then. Huge thanks for everything you guys do for the open source world BTW. |
#1270 and #1273 need to be rolled back as they are causing issues for some windows users.
The text was updated successfully, but these errors were encountered: