Skip to content

Conversation

dgryski
Copy link
Member

@dgryski dgryski commented Sep 26, 2025

No description provided.

@dgryski dgryski force-pushed the dgryski/image-opto branch 3 times, most recently from 3180d65 to 9b07972 Compare September 26, 2025 20:02
@dgryski dgryski force-pushed the dgryski/image-opto branch from 8d78f80 to a094f59 Compare October 2, 2025 19:15
@dgryski dgryski force-pushed the dgryski/image-opto branch from a094f59 to 246da3c Compare October 2, 2025 19:16
@dgryski dgryski marked this pull request as ready for review October 2, 2025 19:26
@dgryski
Copy link
Member Author

dgryski commented Oct 2, 2025

PTAL. Squashed and ready to merge. Docs copied from the Rust SDK.

Copy link
Collaborator

@cee-dub cee-dub left a comment

Choose a reason for hiding this comment

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

Also, FWIW, url.Values.Encode does sort by key.

return nil
}

var rePixelsOrPercentage = regexp.MustCompile("^[0-9]+(.[0-9]+)?p?$")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
var rePixelsOrPercentage = regexp.MustCompile("^[0-9]+(.[0-9]+)?p?$")
var rePixelsOrPercentage = regexp.MustCompile("^[0-9]+(\.[0-9]+)?p?$")

@dgryski dgryski merged commit 2befc18 into main Oct 2, 2025
9 checks passed
@dgryski dgryski deleted the dgryski/image-opto branch October 2, 2025 20:14
@dgryski dgryski mentioned this pull request Oct 2, 2025
// RequestID is the current Fastly request ID
RequestID string

ImageOptimizerOpts *imageopto.Opts

Choose a reason for hiding this comment

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

Do you think it may make sense to name this ImageOptimizerOptions? I ask because this struct already has CacheOptions and DecompressResponseOptions.

The Rust SDK uses ImageOptimizerOptions, and that was also what it was named in the rough design doc we shared.

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.

3 participants