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

Create tmp files with .browserify.js extension #219

Merged
merged 5 commits into from Nov 30, 2018

Conversation

etpinard
Copy link
Contributor

... instead of .browserify to avoid "Invalid file type" logs in karma@3.1.1 from a patch added in karma-runner/karma#3165

Before 4ef50af

npm test > npm-test.base.txt
grep Invalid npm-test.base.txt

# gives
30 11 2018 15:42:10.544:WARN [middleware:karma]: Invalid file type, defaulting to js. browserify
30 11 2018 15:42:30.195:WARN [middleware:karma]: Invalid file type, defaulting to js. browserify
30 11 2018 15:42:31.145:WARN [middleware:karma]: Invalid file type, defaulting to js. browserify

and after

npm test > npm-test.txt
grep Invalid npm-test.txt
# blank!

npm-test.base.txt
npm-test.txt

I could of course add a test for this, if deemed necessary.

... instead of `.browserify` to avoid "Invalid file type" logs
    in karma@3.1.1 from a patch added in:
    karma-runner/karma#3165
@etpinard
Copy link
Contributor Author

etpinard commented Nov 30, 2018

Ah. Looks like karma@3 dropped support for Node@4.

I should probably revert aff7a53, unless karma-browserify is ok with dropping Node@4 support.

Edit: [...] is ok with not being tested on Node@4.

@bendrucker
Copy link
Collaborator

I think it would be ok to drop Node 4 support as part of karma@3 compatibility. It's almost certainly more important to test on karma@latest than continue to support karma@2.

@bendrucker
Copy link
Collaborator

If anything a "no WARN logs are printed" test could be nice. If that's too tricky no test against this change is ok too.

@etpinard
Copy link
Contributor Author

@bendrucker thanks for taking a look at my PR!

I should be able to add a test at some point next week.

@bendrucker
Copy link
Collaborator

Cool! Test is far from required and could even be submitted separately. I would be ok merging this now with:

  • node@4 removed from Travis
  • peerDependencies.karma updated
  • engines.node updated

@bendrucker bendrucker merged commit 496d05c into nikku:master Nov 30, 2018
@bendrucker
Copy link
Collaborator

Thanks! Published as a major (because of the peer + node version changes).

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

2 participants