search

WebReflection / hyperHTML

A Fast & Light Virtual DOM Alternative
ISC License
3.07k stars 112 forks source link

ci: test Node.js 10 #309

Closed DanielRuf closed 5 years ago

DanielRuf commented 5 years ago

stable tests current stable wich is 11 not 10. Let's check 10 too.

Should we also test the other LTS branches like 8 and 6?

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 763

Totals Coverage Status
Change from base Build 762: 0.0%
Covered Lines: 375
Relevant Lines: 380
💛 - Coveralls
WebReflection commented 5 years ago

The only reason node runs is to code cover via Istanbul. This is a client side / DOM / browser library so having anything more than what's running already is meaningless for the target. Accordingly, what is the reason CI should run different version of the non targeted platform? Are you confusing hyperHTML with viperHTML, which runs already various node versions via CI cause node is the target there?

DanielRuf commented 5 years ago

The only reason node runs is to code cover via Istanbul. This is a client side / DOM / browser library so having anything more than what's running already is meaningless for the target.

Not really as the dependencies can break (and also devDeps). Also this affects the contributors. stable is meant to fail early.

WebReflection commented 5 years ago

Dependencies cannot break cause greenkeeper takes care of that, there's really no reason to use any other node version here, it brings nothing but it slows down the CI for no reason. I think this is not necessary/needed

DanielRuf commented 5 years ago

but it slows down the CI for no reason

Not really as 3 or 4 builds run in parallel normally. Also we can use the caches.

DanielRuf commented 5 years ago

Dependencies cannot break cause greenkeeper takes care of that

Just in the current state. I can always trigger a new build and greenkepper would probably not see that. Also a later build can always break.

stable / node is meant as "fail early" solution.

WebReflection commented 5 years ago

Again, this change brings nothing to the library so it makes really no sense. I'm also on vacation and I won't merge something that is not important, neither needed.

DanielRuf commented 5 years ago

Ok, understood.

Then please enjoy your vacation and ignore my stuff. No hurry.