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.

Sign up for GitHub

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

Jump to bottom

ci: test on Node.js 6, 8, 10 and 11 #525

Closed
wants to merge 1 commit into from
Closed

ci: test on Node.js 6, 8, 10 and 11 #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .appveyor.yml
View file
Open in desktop
Expand Up @@ -4,9 +4,10 @@

environment:
matrix:
- nodejs_version: '11'
- nodejs_version: '10'
- nodejs_version: '9'
- nodejs_version: '8'
- nodejs_version: '6'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package doesn't support Node 6. Minimum requirement is Node 8.6

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, then it should be made clear in the engines field. Otherwise people might run it on older versions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#515

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an ideal solution tbh, yarn --ignore engines is recommended because the current solution provides no consistent behavior across different platforms. What if you run a CI runner on your pc?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by a CI runner?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitLab runner, SSH runner, tools like Surf / Deployer, ... which we use in CI and locally.

Copy link
Collaborator

@okonet okonet Oct 31, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I’m not following. I think the solution is okay since 🚫💩 lint-staged isn’t supposed to be used on CI so it shouldn’t break it. We have safety nets built in for runtime.


matrix:
fast_finish: true
Expand Down
3 changes: 2 additions & 1 deletion .travis.yml
View file
Open in desktop
Expand Up @@ -4,9 +4,10 @@

language: node_js
node_js:
- 'node'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean? Which version is it going to be?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node is the current stable which is 11 at the moment.

- '10'
- '9'
- '8'
- '6'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, please remove.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After adding engines to package.json, yes.


before_install: yarn global add greenkeeper-lockfile@1
install: yarn install
Expand Down