foundation / panini

A super simple flat file generator.
Other
592 stars 104 forks source link

tests: use npm ci #165

Closed DanielRuf closed 4 years ago

DanielRuf commented 6 years ago

npm ci is much faster and is meant for CI. This PR is meant for evaluating the new ci command and testig the performance and compare the logs of Travis CI.

https://docs.npmjs.com/cli/ci http://blog.npmjs.org/post/171556855892/introducing-npm-ci-for-faster-more-reliable

gakimball commented 6 years ago

I'd recommend continuing to test on Node 0.12, since the library still supports it. The next major version will probably be Node 6+ only, since Node 4 is EOL at the end of the month.

DanielRuf commented 6 years ago

Right. We can wait with the merge. Will compare the results tomorrow.

ncoden commented 6 years ago

We can also use npm install for v0.12 and npm ci for others versions. See the matrix option like in the Foundation travis config

ncoden commented 6 years ago
matrix:
  include:
  - node_js: "0.12"
    env: ZF_NPM_CI=false
  - node_js: "4"
    env: ZF_NPM_CI=true
  - node_js: "5"
    env: ZF_NPM_CI=false
  - node_js: "6"
    env: ZF_NPM_CI=true
  - node_js: "7"
    env: ZF_NPM_CI=false
  - node_js: "8"
    env: ZF_NPM_CI=true
  - node_js: "9"
    env: ZF_NPM_CI=false

before_install:
- if [ $ZF_NPM_CI = true ]; then npm install -g npm@latest; fi
install:
- if [ $ZF_NPM_CI = true ]; then npm ci; else npm install; fi
DanielRuf commented 6 years ago

Is there a specific reason for true / false / true / false / true ... (the npm ci / i pattern)? =)

ncoden commented 6 years ago

Is there a specific reason for true / false / true / false / true ... (the npm ci / i pattern)? =)

The new NPM version is only supported on latest and LTS node versions.

DanielRuf commented 6 years ago

Do we even need to check all versions even the versions which reached their EOL?

gakimball commented 6 years ago

Do we even need to check all versions even the versions which reached their EOL?

Agreed, I'd say drop the non-LTS versions: 5, 7, and 9. The really old versions need to stick around for compatibility reasons.

ncoden commented 6 years ago

The really old versions need to stick around for compatibility reasons.

I'm not sure to understand. Do we need to test 0.12 and 4 in travis ?

gakimball commented 6 years ago

I'm not sure to understand. Do we need to test 0.12 and 4 in travis ?

It's definitely weird because this library supports a super old version of Node, but typically I would only test a Node library against actively supported LTS versions. Since it has to support 0.12, I'd say throw in 4 for good measure it should also support 4, which was an LTS release, even though it's not maintained anymore. So my suggestion would be 0.12, 4, 6, 8, and 10.

The next major version of Panini will only support 6 and up.

ncoden commented 6 years ago

I would only test a Node library against actively supported LTS versions

I would test a library against the version we want to support following the maintanance cost. For now I have nothing against keeping an official 0.12+ support and testing on it too.

If we start to add new features and 0.12 is too anoying for that, we will officialy drop it and stop testing on it.

So my suggestion would be 0.12, 4, 6, 8, and 10. The next major version of Panini will only support 6 and up.

Agreed

So we can would with what I proposed at https://github.com/zurb/panini/pull/165#issuecomment-383335370 ?

gakimball commented 6 years ago

I would test a library against the version we want to support following the maintanance cost. For now I have nothing against keeping an official 0.12+ support and testing on it too. If we start to add new features and 0.12 is too anoying for that, we will officialy drop it and stop testing on it.

From my perspective, v1 of Panini is basically EOL. v2 is pretty far along, but I've had limited time to work on it recently.

I don't think v1 ever had its Node requirements spelled out—it's not in the readme, and there's no engines field in the package.json. Even so, since it supports 0.12, I'd prefer to not drop it without a major version bump.

Continuing to support 0.12 for this version won't be a hassle though, since like I said, I don't think anything big is going to happen to Panini v1 feature-wise.