flowjs / flow.js

A JavaScript library providing multiple simultaneous, stable, fault-tolerant and resumable/restartable file uploads via the HTML5 File API.
Other
2.96k stars 346 forks source link

Ability to use ES6: rollup+babel transpilation #309

Closed drzraf closed 4 years ago

drzraf commented 4 years ago

This change gives the opportunity to use ES6 for src/flow.js

Also update to karma-coverage v2 needed for async support as discussed and anticipated in https://github.com/flowjs/flow.js/pull/304.

AidasK commented 4 years ago

You are doing a huge upgrade and that's great. Would you like to be added as a maintainer of this project?

drzraf commented 4 years ago

As long as someone keeps reviewing PR, I've no problem with this but I can't guarantee long-term involvement.

Still, about that specific PR, I don't understand why the CI fails, is this something you could fix to get this green?

AidasK commented 4 years ago

It looks like it can't generate karma coverage report because coverage plugin is missing. Though all the tests pass. You can see the log here: https://travis-ci.org/github/flowjs/flow.js/jobs/723863503

drzraf commented 4 years ago

I guess the plugin is not loaded (anymore) because node can't parse node_modules/karma-sauce-launcher. Updating the version of node in .travis.yml may do it?

AidasK commented 4 years ago

https://travis-ci.org/github/flowjs/flow.js/jobs/724987732 now we are getting different error

/home/travis/.nvm/versions/node/v14.9.0/lib/node_modules/codeclimate-test-reporter/formatter.js:33
      throw parseError;
      ^
Unknown input coverage format
(Use `node --trace-uncaught ...` to show where the exception was thrown)
The command "$TRAVIS_BUILD_DIR/travis.sh" exited with 1.

It looks like coverage was created, but with different format or empty file.

AidasK commented 4 years ago

can you test if grunt karma:coverage generates a valid coverage report? e.g. it should create coverage/*/lcov.info files

drzraf commented 4 years ago

Could you set your CC_TEST_REPORTER_ID if different from CODECLIMATE_REPO_TOKEN? Note: These variables (like SAUCE_ACCESS_KEY) should probably be secret and stored inside GitHub configuration. You may also want to remove some configuration from the browsers sauce test against.

It's not doing coverage reporting (0-size file). But I don't understand which tool is actually in charge of code instrumentation?

AidasK commented 4 years ago

Great job, it works! I have added you to the maintainers of this project. Once finished, your branch will be released as v3.0.0 flowjs since it supports promises. Please don't forget documentation too 🥇

drzraf commented 4 years ago

Does not work. lcov.info is 0 bytes.

drzraf commented 4 years ago

code-coverage fixed. Should I let you squash-merge on -master? Or on a v3 branch if you prefer such solution the time being?

AidasK commented 4 years ago

Yes please, squash it and push it to v3 branch. Feel free to do it yourself, you have all the permissions you need. We need some time to release it to master once it's tested.

drzraf commented 4 years ago

Note: The update to karma-coverage v2 caused start --single-run=false to stop functioning. (https://github.com/karma-runner/karma-sauce-launcher/issues/217) Will fix this in a subsequent commit.