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

NPM scripts as Make alternative for Windows #833

Closed
wants to merge 2 commits into from

Conversation

DaAwesomeP
Copy link

I know there's been a lot of debate here about this, so I tried to do this without affecting the workflows of people who want to use Make: I didn't touch the Makefile or Travis file. Make on Windows is annoying to set up (NodeJS also doesn't work well with MSYS2), and I don't see why not use NPM scripts in that case.

They can be used in the following way:

npm test
npm make-test  # will actually just run "make test"
npm run test-cov
npm run test-travis
npm run lint

@codecov-io
Copy link

codecov-io commented Oct 16, 2016

Current coverage is 100% (diff: 100%)

Merging #833 into v2.x will not change coverage

@@           v2.x   #833   diff @@
==================================
  Files         4      4          
  Lines       418    418          
  Methods      80     80          
  Messages      0      0          
  Branches    100    100          
==================================
  Hits        418    418          
  Misses        0      0          
  Partials      0      0          

Powered by Codecov. Last update 2a16426...14d62b0

@jonathanong
Copy link
Member

we had a discussion about this and we were on the fence

@DaAwesomeP
Copy link
Author

Yeah I saw #560. That's why I didn't replace Make, but rather added NPM alongside it. Yes, it's redundant to have two methods of running the scripts, but this makes more sense for average Joe either on Windows or who may not have Make installed.

@jonathanong
Copy link
Member

i might be switching to windows so i might end up merging windows support. however, i wouldn't merge this in its current form since 300-character commands are not reasonable. we need to move them to mocha.opts or something

@dougwilson
Copy link
Contributor

Why would "average Joe" run Koa's test suite though?

To contribute to Koa.

@DaAwesomeP
Copy link
Author

DaAwesomeP commented Oct 29, 2016

300-character commands are not reasonable. we need to move them to mocha.opts or something

@jonathanong I definitely agree. I'll add a mocha.opts file to the test/ directory. Should I edit the Make rules as well to use this? I would remove the REQUIRED and TESTS variables and the --bail option on each rule altogether.

@jonathanong jonathanong changed the base branch from v2.x to master February 25, 2017 06:26
@jonathanong
Copy link
Member

#703 gonna switch over to jest, which will make the test command 1 line and will make this easier to do!

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.

4 participants