adds happiness, resolves #224

[dcousens]

Currently the tests are failing because eslint explodes.

[bruderstein]

You don't need to do npm run coveralls and npm run unit, coveralls runs the tests, and fails if the tests fail.

[dcousens]

@bruderstein perhaps then coveralls is doing too much?
I'd expect that target to run all the way even if the tests fail?
No?

If a test fails, that doesn't equate 0% or failed coverage.

[bruderstein]

To test coverage, we run the tests. That's what coverage is 😄

If we make this pass even if the tests fail (just add an || true), then the tests run twice, which is just extra work with no benefit.

[dcousens]

@bruderstein I've also found keeping coveralls separate helps when coveralls fails, which with my experience in https://github.com/bitcoinjs/bitcoinjs-lib/, happens often.

For comparison, our scripts can be found here.
I agree though, it does seem a bit redundant, and I'm on the fence about whether or not to change it.

[dcousens]

@bruderstein why not just istanbul cover ... and | coveralls by the way?

[bruderstein]

I took that from the coveralls site, if there's a better way, let's have it.

On Thu, Aug 13, 2015 at 8:57 AM, Daniel Cousens [email protected]
wrote:

In package.json
#373 (comment):

@@ -48,11 +49,12 @@
"cover": "istanbul cover node_modules/.bin/_mocha -- -u exports --compilers js:babel/register -R spec",
"coveralls": "NODE_ENV=test ./node_modules/.bin/istanbul cover ./node_modules/mocha/bin/_mocha --report lcovonly -- -u exports --compilers js:babel/register -R spec && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js",

@bruderstein https://github.com/bruderstein why not just | coveralls by
the way?

—
Reply to this email directly or view it on GitHub
https://github.com/JedWatson/react-select/pull/373/files#r36945543.

[dcousens]

@bruderstein changed in 65358b5

[dcousens]

I'll leave this one to you @bruderstein