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

Use npm-run-scripts for commandline tooling #169

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

BigBlueHat
Copy link
Contributor

Useful for Windows development (hence the new cross-env dependency).

Makefile is still there in case deploy/host scripts depend on it...though
it could in turn use the npm run scripts.

The commands match the Makefile commands, so:

$ make test-node
$ # is the same as
$ npm run test-node

Let me know if it needs tweaks!

Useful for Windows development (hence the new `cross-env` dependency).

Makefile is still there in case deploy/host scripts depend on it...though
it could in turn use the npm run scripts.
@BigBlueHat
Copy link
Contributor Author

Also, I should note that I've tested this in the Windows Linux Sub-System (lxss; aka "ubuntu in windows"), MingW, and PowerShell.

Seems they do not detect bash on Windows (mingw) environments.
@BigBlueHat
Copy link
Contributor Author

Found and fixed a path issue when running in bash (mingw) in Windows. Should be good to go (completely) in all those scenarios now.

@adambro
Copy link

adambro commented Aug 9, 2017

Just an idea: Makefile will be just a way to run NPM scripts, so it could be replaced by fakefile

@dlongley
Copy link
Member

dlongley commented Aug 9, 2017

I would like to close this out, I think #194 will supersede everything here -- so we shouldn't put any more effort into this PR. @davidlehn can you confirm?

@davidlehn davidlehn added this to the v0.5.0 milestone Aug 9, 2017
@davidlehn
Copy link
Member

davidlehn commented Aug 9, 2017

The Makefile will be gone. Will have to check new commands for portability. I'm guessing things like ${REPORTER:-spec} that are sh specific might have issues. So cross-env or a minor rework might be needed. It's just nice to be able to do REPORTER=nyan npm t when the need arises.

@davidlehn
Copy link
Member

The code has changed quite a bit. This patch needs to be updated if it's still an issue.

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

4 participants