-
-
Notifications
You must be signed in to change notification settings - Fork 30
Conversation
* convenient for our async/await model. | ||
* | ||
* @param {Buffer|string} data - binary image buffer or base64-encoded image | ||
* string |
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.
missing return description
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.
fixed
lib/image-util.js
Outdated
* @param {Buffer|string} data - binary image buffer or base64-encoded image | ||
* string | ||
*/ | ||
async function getJimpImage (data) { |
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.
Perhaps toJimpImage
?
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.
hmm, if this were a method on an object i would like that, but since it's just a bare function that feels confusing to me. open to other ideas though.
lib/image-util.js
Outdated
async function getJimpImage (data) { | ||
return await new B((resolve, reject) => { | ||
if (!_.isString(data) && !_.isBuffer(data)) { | ||
throw new Error("Must initialize jimp object with string or buffer"); |
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.
Probably better to reject rather than throw in here, so as not to mix metaphors.
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, good idea
published in |
was finding that i was having to do lots of bluebird messiness just to use jimp in a few different places. this PR puts all that messiness in one place.