Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(s3): new parameter "format" for png images #5

Merged
merged 1 commit into from Feb 13, 2018
Merged

Conversation

wl00887404
Copy link
Contributor

@wl00887404 wl00887404 commented Feb 7, 2018

tmotx/tickets#42

Related: https://github.com/tmotx/platform/pull/367

@@ -52,10 +52,11 @@ export default class StorageS3 {
return this;
}

async transform(fromKey, toKey, transform = sharp().jpeg()) {
async transform(fromKey, toKey, transform = sharp(), format) {
Copy link
Contributor

Choose a reason for hiding this comment

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

args 不應該 required, required, option, required

Copy link
Contributor

@yutin1987 yutin1987 left a comment

Choose a reason for hiding this comment

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

寫一個 toFormat 保留 transform 的彈性

@@ -52,10 +52,11 @@ export default class StorageS3 {
return this;
}

async transform(fromKey, toKey, transform = sharp().jpeg()) {
async transform(fromKey, toKey, format, transform = sharp()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

transform() 不需要,format 可以維持 transform 的彈性

await this.s3.copyObject({ CopySource: `${this.bucket}/uploader/${key}`, Key: `media/${toKey}` }).promise();
await this.transform(`media/${toKey}`, `media/${toKey}_cover`);
await this.transform(`media/${toKey}`, `media/${toKey}_cover`, format);
Copy link
Contributor

Choose a reason for hiding this comment

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

await this.transform(media/${toKey}, media/${toKey}_cover, toFormat(sharp(), format));

const cacheName = `cache/${key}_cover_${[width, height].join('x')}`;

try {
await this.s3.headObject({ Key: cacheName }).promise();
} catch (e) {
await this.transform(`media/${key}_cover`, cacheName, sharp().resize(width, height).jpeg());
await this.transform(`media/${key}_cover`, cacheName, format, sharp().resize(width, height));
Copy link
Contributor

Choose a reason for hiding this comment

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

await this.transform(media/${key}_cover, cacheName, toFormat(sharp().resize(width, height), format));

@yutin1987
Copy link
Contributor

@wl00887404 測試需要可以 pass

@wl00887404 wl00887404 closed this Feb 13, 2018
@wl00887404 wl00887404 reopened this Feb 13, 2018
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 99.94% when pulling 7db11d6 on pngQAQ into 6d1e9d9 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 99.94% when pulling 7db11d6 on pngQAQ into 6d1e9d9 on master.

@yutin1987 yutin1987 merged commit 9d4a146 into master Feb 13, 2018
@yutin1987 yutin1987 deleted the pngQAQ branch February 13, 2018 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants