Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

chore: Modernize, update dependencies #9

Merged
merged 6 commits into from Jan 12, 2018
Merged

chore: Modernize, update dependencies #9

merged 6 commits into from Jan 12, 2018

Conversation

aearly
Copy link
Contributor

@aearly aearly commented Jan 12, 2018

We still get a warning about a deprecated node-uuid through handlebars-async, but that project hasn't been touched in 4 years...

@aearly aearly requested a review from bcoe January 12, 2018 20:51
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9643fc2 on modernize into d3ddcd7 on master.

Copy link

@chrisdickinson chrisdickinson left a comment

Choose a reason for hiding this comment

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

This looks great, much cleaner! ✨

Am I correct in thinking that we'll have to upgrade the email-queue service to node 8 as part of this?

const Promise = require('bluebird')
const path = require('path')
const _ = require('lodash')
const fs = require('fs')

Choose a reason for hiding this comment

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

if (!path) return P.cast(null);

return new P(function(resolve, reject) {
fs.readFile(path, 'utf-8', function(err, source) {

Choose a reason for hiding this comment

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

(Oh odd, I did not know that node supported utf-8 in addition to utf8.)

Copy link

@jefflembeck jefflembeck left a comment

Choose a reason for hiding this comment

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

Hot damn! nice

@aearly
Copy link
Contributor Author

aearly commented Jan 12, 2018

Am I correct in thinking that we'll have to upgrade the email-queue service to node 8 as part of this?

Not initially. This will be used directly by wubwub. If/when we upgrade email-queue to use this version as well, we will also have to upgrade it to node 8.

This module is effectively a mustache and monocle atop SES, so there's little harm in keeping around an old version.

@aearly aearly merged commit 423c5ba into master Jan 12, 2018
@aearly aearly removed the request for review from bcoe January 12, 2018 21:36
@aearly aearly deleted the modernize branch January 12, 2018 21:36
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.

None yet

4 participants