crossfilter / reductio

Reductio: Crossfilter grouping
Other
248 stars 42 forks source link

Es6module #60

Closed christophe-g closed 4 years ago

christophe-g commented 5 years ago

Port to ES6 module, same exercice as https://github.com/crossfilter/crossfilter/pull/142

Tests are passing against the build file (reductio.min.js). Struggling to make them pass against src files due to karma/jasmine having trouble properly interpreting import statement.

Travis build failing, potentially due to old node engine (0.10). Running fine locally on my machine (node 10.16.3).

christophe-g commented 4 years ago

Dear @esjewett

Now that crossfilter runs Es6 modules, any chance to get this reviewed ? Thanks, C.

esjewett commented 4 years ago

@christophe-g Yup, I can look at this. Is this ready to review?

christophe-g commented 4 years ago

@esjewett - excellent thanks.

Yes it is from my perspective. I am using this branch without apparent problems.

As said above, test seems to need more recent nodejs than the one in travis.

esjewett commented 4 years ago

@christophe-g Ok, looking good, and confirmed that the tests are passing.

Two questions:

christophe-g commented 4 years ago

That was quick !

Are you able to update the Travis node version in the .travis.yml file in your branch, or does that not work?

Done, travis success.

What would we expect this to break? I'm guessing this will be a major version bump as it will break pretty much all existing usage of the library?

It will break ie. Yes it should be a major version bump. I suggest we get inspiration from latest crossfilter release (https://github.com/crossfilter/crossfilter/releases/tag/1.5.0) for release note.

esjewett commented 4 years ago

Excellent - thank you. I will try to get this merged and released today or this weekend.