Skip to content
This repository has been archived by the owner on Jul 12, 2019. It is now read-only.

Bump versions of dependencies and Node.js #94

Merged
merged 3 commits into from
Jun 19, 2018
Merged

Bump versions of dependencies and Node.js #94

merged 3 commits into from
Jun 19, 2018

Conversation

kola-er
Copy link
Contributor

@kola-er kola-er commented Jun 12, 2018

Add support for Node.js 8.10.

@kola-er
Copy link
Contributor Author

kola-er commented Jun 14, 2018

Until [email protected] is published, the test would keep failing. Waiting on zapier/zapier-platform-schema#48 to be merged and published on npm.

@kola-er kola-er changed the title WIP: Bump versions of dependencies and Node.js Bump versions of dependencies and Node.js Jun 14, 2018
@kola-er kola-er requested a review from xavdid June 14, 2018 00:32
Copy link
Contributor

@xavdid xavdid left a comment

Choose a reason for hiding this comment

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

the only blocker is the schema change, assuming you've checked out that fetch bump

.travis.yml Outdated
@@ -2,7 +2,7 @@ language: node_js
node_js:
- 6.10.3
- '7'
Copy link
Contributor

Choose a reason for hiding this comment

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

we can remove 7, it'll never be on AWS.

we could also add '10' just to see if there's anything bad (eventually) coming down the pipe

package.json Outdated
"node-fetch": "1.7.1",
"zapier-platform-schema": "6.1.0"
"node-fetch": "2.1.2",
"zapier-platform-schema": "7.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

this gets bumped automatically on release, so we shouldn't change it manually here.

package.json Outdated
@@ -36,8 +36,8 @@
"dotenv": "5.0.1",
"form-data": "2.2.0",
"lodash": "4.17.10",
"node-fetch": "1.7.1",
"zapier-platform-schema": "6.1.0"
"node-fetch": "2.1.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

is this is a safe change to make? Seems like there are some actual incompatibilities here, though I don't know if we're using any of these one way or the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking around the changelog, I don't see any 🔥using 2.1.2. This part of v2.0.0 general changes pushed me to bump it though:

Enhance: start testing on Node.js v4.x, v6.x, v8.x LTS, as well as v9.x stable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be on a safe side, let's stick with 1.7.1 in this release since it isn't broken running on Node.js 8.10.0.

@kola-er kola-er merged commit d9bac89 into master Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants