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.

Sign up for GitHub

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

Jump to bottom

Select MIME type to use in frontend content based on file size #243

Merged
merged 10 commits into from
Mar 31, 2022
Merged

Select MIME type to use in frontend content based on file size #243

merged 10 commits into from
Mar 31, 2022

Conversation

jjgrainger
Copy link
Contributor

@jjgrainger jjgrainger commented Mar 21, 2022 •
edited
Loading

Summary

Fixes #186

Relevant technical choices

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@jjgrainger jjgrainger requested a review from mitogh March 21, 2022 11:10
@jjgrainger jjgrainger self-assigned this Mar 21, 2022
@jjgrainger jjgrainger added [Type] Enhancement A suggestion for improvement of an existing feature [Focus] Images no milestone PRs that do not have a defined milestone for release labels Mar 21, 2022
@jjgrainger jjgrainger changed the title Feature/186 select smaller image size from sources update Select smaller image size from sources update Mar 21, 2022
@jjgrainger jjgrainger mentioned this pull request Mar 21, 2022
Closed
3 tasks
@jjgrainger jjgrainger marked this pull request as ready for review March 21, 2022 16:30
mitogh
mitogh reviewed Mar 21, 2022
View reviewed changes
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
@jjgrainger jjgrainger mentioned this pull request Mar 22, 2022
Closed
eugene-manuilov
eugene-manuilov requested changes Mar 22, 2022
View reviewed changes
Copy link
Contributor

@eugene-manuilov eugene-manuilov left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me. Added a few comments with a few nitpicks, nothing major. Also, do we need to target this PR against the release/1.0.0-beta.x branch?

modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
@jjgrainger
Copy link
Contributor Author

Mostly looks good to me. Added a few comments with a few nitpicks, nothing major. Also, do we need to target this PR against the release/1.0.0-beta.x branch?

I agree, though not sure which release branch it should target? There are currently 3.

eugene-manuilov
eugene-manuilov requested changes Mar 23, 2022
View reviewed changes
Copy link
Contributor

@eugene-manuilov eugene-manuilov left a comment

Choose a reason for hiding this comment

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

Almost looks good to me. Just added a few nitpicks.

modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
mitogh
mitogh reviewed Mar 23, 2022
View reviewed changes
Copy link
Member

@mitogh mitogh left a comment

Choose a reason for hiding this comment

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

Great work, just some minor suggestions / questions.

tests/modules/images/webp-uploads/webp-uploads-test.php Outdated Show resolved Hide resolved
tests/modules/images/webp-uploads/webp-uploads-test.php Show resolved Hide resolved
tests/modules/images/webp-uploads/webp-uploads-test.php Outdated Show resolved Hide resolved
tests/modules/images/webp-uploads/webp-uploads-test.php Show resolved Hide resolved
tests/modules/images/webp-uploads/webp-uploads-test.php Outdated Show resolved Hide resolved
@jjgrainger jjgrainger changed the base branch from trunk to release/1.0.0-rc.1 March 30, 2022 11:07
@jjgrainger jjgrainger requested a review from felixarntz as a code owner March 30, 2022 11:13
@mitogh mitogh changed the base branch from release/1.0.0-rc.1 to trunk March 30, 2022 16:27
@mitogh mitogh added this to the 1.0.0 milestone Mar 31, 2022
@mitogh mitogh removed the no milestone PRs that do not have a defined milestone for release label Mar 31, 2022
@mitogh mitogh merged commit 7930608 into trunk Mar 31, 2022
@mitogh mitogh deleted the feature/186-select-smaller-image-size-from-sources-update branch March 31, 2022 23:59
@felixarntz felixarntz changed the title Select smaller image size from sources update Select MIME type to use in frontend based on file size Apr 18, 2022
@felixarntz felixarntz changed the title Select MIME type to use in frontend based on file size Select MIME type to use in frontend content based on file size Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eugene-manuilov eugene-manuilov eugene-manuilov approved these changes

@mitogh mitogh mitogh approved these changes

@adamsilverstein adamsilverstein Awaiting requested review from adamsilverstein

@getsource getsource Awaiting requested review from getsource

@felixarntz felixarntz Awaiting requested review from felixarntz

Assignees

@jjgrainger jjgrainger

Labels
[Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.

Select the smaller image size from the list of available sources
3 participants
@jjgrainger @eugene-manuilov @mitogh