Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds support for the WebP image format to the Reticulum distributed image server. The implementation correctly updates the necessary encoder mappings and MIME type associations to enable WebP support throughout the system. According to issue #57, the underlying image processing library (bimg/libvips) already handles WebP files properly, so the changes primarily involve updating extension and MIME type mappings.
Changes:
- Added WebP encoder using the
nativewebpGo library - Updated MIME type and extension mappings to include WebP support
- Added test coverage for WebP image handling
- Applied code formatting improvements (struct field alignment, .PHONY directives)
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| encode.go | Added encodeWEBP function and registered it in extencoders map |
| views.go | Added WebP to mimeexts and extmimes mappings |
| image_specifier_test.go | Added Test_Webp to verify WebP extension parsing |
| views_test.go | Updated test context and added WebP test case |
| go.mod | Added nativewebp and golang.org/x/image dependencies |
| go.sum | Updated with checksums for new dependencies |
| config.go | Applied struct field alignment formatting |
| worker.go | Applied minor formatting improvements |
| Makefile | Added .PHONY directives for build targets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
encode.go
Outdated
| return gif.Encode(out, in, &gifOptions) | ||
| } | ||
|
|
||
| func encodeWEBP(out io.Writer, in image.Image) error { |
There was a problem hiding this comment.
The function name encodeWEBP is inconsistent with the naming convention used for other encoder functions in this file. Other encoders use PascalCase (encodeJPEG, encodeGIF, encodePNG), but this uses all caps for WEBP. Consider renaming to encodeWebp or encodeWebP to match the existing pattern.
encode.go
Outdated
| ".jpg": encodeJPEG, | ||
| ".gif": encodeGIF, | ||
| ".png": encodePNG, | ||
| ".webp": encodeWEBP, |
There was a problem hiding this comment.
This reference should be updated to match the corrected encoder function name (either encodeWebp or encodeWebP) for consistency with the naming convention.
| {"/image/0051ec03fb813e8731224ee06feee7c828ceae22//image.jpg", http.StatusNotFound}, | ||
| {"/image/0051ec03fb813e8731224ee06feee7c828ceae22/100s/", http.StatusNotFound}, | ||
| {"/image/0051ec03fb813e8731224ee06feee7c828ceae22/100s/image.jpeg", http.StatusMovedPermanently}, | ||
| {"/image/0051ec03fb813e8731224ee06feee7c828ceae22/100s/image.webp", http.StatusOK}, |
There was a problem hiding this comment.
The original test case for JPEG extension redirect behavior has been removed. The previous test verified that .jpeg files get redirected to .jpg (HTTP 301), which is an important behavior defined in views.go. Consider adding back the JPEG redirect test case in addition to the new WebP test case to maintain test coverage.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
- Rename encodeWEBP to encodeWebP for consistency - Update extencoders map reference - Add back JPEG redirect test case Co-authored-by: thraxil <7821+thraxil@users.noreply.github.com>
Co-authored-by: thraxil <7821+thraxil@users.noreply.github.com>
should close #57