-
Notifications
You must be signed in to change notification settings - Fork 0
Move to Node 8 / npm 5, update packages and cleanup #215
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on the cleanup. 👍
package.json
Outdated
"aws-sdk": "^2.45.0", | ||
"bluebird": "^3.5.0", | ||
"boom": "^4.3.1", | ||
"auth0-js": "^8.7.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used somewhere? Same thing for its @types package
package.json
Outdated
"yamljs": "^0.2.10" | ||
}, | ||
"devDependencies": { | ||
"@types/auth0-js": "^8.6.0", | ||
"@types/bluebird": "^3.5.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly, but knex's typings depend on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since @types/knex depends on it, we don't need to add it to our dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I thought it was missing from the @types/knex's package.json, but apparently when I ran npm rm @types/bluebird
I should've run npm install
afterwards. Removed now.
src/operations/operations-module.ts
Outdated
if (existing === null) { | ||
throw Boom.badGateway(); | ||
} | ||
const mappedExisting = (existing as any).map((item: any) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the reason you can't do existing.map()
is that the Promise.all
call above is incorrect. Shouldn't it be:
const [ expected, existing ] = await Promise.all([
this.getProjectDeploymentActivity(projectId),
this.activityModule.getProjectActivity(projectId),
]);
?
Use util.promisify from Node Delete obsolete Route53Updater Remove obsolete aws-sdk typings and promisifications Compile to es2017 to take advantage of Node 8's native async/await support Include package-lock.json with the upgrade to npm 5
Upgrade circle to Node 8 as well
ea3446a
to
30399fa
Compare
👍 |
Use
util.promisify
from Node 8 and remove the self-coded implementationDelete obsolete Route53Updater
Remove obsolete aws-sdk typings and promisifications
Compile to es2017 to take advantage of Node 8's native async/await support
Include package-lock.json with the upgrade to npm 5
TODO