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.

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

CI: Change Node.js to v6 #34

Merged
merged 1 commit into from
Nov 25, 2018
Merged

CI: Change Node.js to v6 #34

merged 1 commit into from
Nov 25, 2018

Conversation

mike-north
Copy link
Contributor

This library depends on [email protected], which requires node 6 or above

@mike-north mike-north merged commit 43547b4 into master Nov 25, 2018
@Turbo87
Copy link
Collaborator

Turbo87 commented Nov 25, 2018

@mike-north please don't merge code without review. This PR is missing the engine bump in package.json.

@Turbo87 Turbo87 deleted the mike-north-patch-1 branch November 25, 2018 18:11
@mike-north
Copy link
Contributor Author

@Turbo87 I'll refrain from merging in these pure CI config fixes, and any other changes in the future. Several simplabs pipelines have been failing on master for up to 1 month -- thought that these changes would be welcomed and a "no brainer" thing to go in.

@Turbo87
Copy link
Collaborator

Turbo87 commented Nov 26, 2018

thought that these changes would be welcomed and a "no brainer" thing to go in.

don't get me wrong, these PRs are very welcome, just please ping any of us to get a review before merging yourself 🙏

@Turbo87 Turbo87 changed the title chore: update CI node version to 6 CI: Change Node.js to v6 Nov 26, 2018
@Turbo87
Copy link
Collaborator

Turbo87 commented Nov 27, 2018

This library depends on [email protected]

I've just checked this. The problem is that we use [email protected] as a dev dependency, which depends on testem@2 which depends on execa@1. The addon itself should still work fine on Node 4 though, but I'll go ahead and bump the supported version in the package.json (#35) to unblock some of the dependabot PRs and then release a v0.4.0.

@mike-north
Copy link
Contributor Author

The problem is that we use [email protected] as a dev dependency, which depends on testem@2 which depends on execa@1

This was my original basis for only bumping the version up in CI, and leaving package.json engines alone. I don't believe that we need to force any consumer off node4, although we do need to test on node6+.

@Turbo87
Copy link
Collaborator

Turbo87 commented Nov 27, 2018

yeah, but it seems unfortunate to test on a higher Node version than what we support. an alternative would be to revert to [email protected] (I think...), which still uses testem@1 and does not have that execa@1 dependency, but dropping Node 4 support is probably the best way forward anyway. For users that still need Node 4 support I've added a v0.3.x branch to the repo, where we could do the CLI downgrade if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants