-
Notifications
You must be signed in to change notification settings - Fork 4
Conversation
9ab19f7
to
3fd7dee
Compare
// When cropping, we size the image first to fix completely within the bounding box | ||
// Then we crop the requested size, and center to the center of the image | ||
const crop = async(client, params) => { | ||
client = client.resize(params.width, params.height, '^'); |
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.
what is that ^
? needs a comment
} | ||
} | ||
|
||
// Check if the minimum is ste |
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.
dafuq is ste
?
nice dude! |
result.type = req.params.format.toLowerCase(); | ||
result.mime = getMimeFromExtension(result.type); | ||
} | ||
if (req.query.fit && ALLOWED_FITS.includes(req.query.fit)) { |
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.
needs a .toLowerCase as well
Todo:
|
}); | ||
const S3 = new AWS.S3(); | ||
|
||
const uploadPromise = (client, params) => { |
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.
uploadPromise
isn't a promise. I'd just call it upload
.
Nice work! I have quite a few comments, but all are minor. So definitely 👍 |
@TiddoLangerak @wouter-willems @joostverdoorn Can I haz a first 👀 ? Did a small Tiddo and wanted to rewrite this to proper logic. This way the code does not hurt my eyes anymore.
The diff is terrible though, so you might be better off checking the branch itself.