Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved issues remain with libvips provisioning, post-conversion quota enforcement, and synchronous batch processing.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Adds default-enabled AVIF conversion for web batch uploads using image_processing/Vips.
Changes:
- Adds AVIF conversion and metadata handling.
- Wires the option through the controller and upload UI.
- Adds dependencies and service/controller tests.
| File | Summary |
|---|---|
test/services/batch_upload_service_test.rb |
Tests conversion behavior. |
test/controllers/uploads_batch_test.rb |
Tests UI and controller integration. |
Gemfile.lock |
Locks image-processing dependencies. |
Gemfile |
Adds the image-processing dependency. |
app/services/batch_upload_service.rb |
Implements AVIF conversion. |
app/controllers/uploads_controller.rb |
Passes the conversion option. |
app/components/uploads/index.rb |
Adds the conversion checkbox. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def convert_to_avif(file) | ||
| ImageProcessing::Vips | ||
| .source(file.tempfile.path) | ||
| .convert(:avif) | ||
| .saver(Q: 100) | ||
| .call |
There was a problem hiding this comment.
seems pointless. avif converesion is not slow by any means and adding a job for this adds additional overhead more than what would be needed.
awaiting further review
AI wasnt used to make this pull request
Resolves cdn-dev
uses previously commented dep
image_processinggem to convert images to avif formatwasnt really sure where to put the option for automatic conversion, for now it is under the upload button
