Skip to content

fix: svg files should be compressed#3486

Merged
robjtede merged 4 commits into
actix:masterfrom
Degubi:fix-svg-compress
May 10, 2025
Merged

fix: svg files should be compressed#3486
robjtede merged 4 commits into
actix:masterfrom
Degubi:fix-svg-compress

Conversation

@Degubi

@Degubi Degubi commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

PR Type

Bug Fix

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

#3075 removed compression for image & video files, because they usually come pre-compressed.
SVG (image/svg+xml) files are exceptions (IMO), because they're text based, so they could benefit from being served as compressed assets.

Note 1: I wasn't sure whether to add a test or not.
Note 2: I wasn't able to test this change because I was getting a wall of seemingly unrelated 'unresolved imports' errors in my project after switching to my fork in the Cargo.toml.

@robjtede robjtede added A-files project: actix-files B-semver-patch A-web project: actix-web and removed A-files project: actix-files labels Oct 16, 2024
@Degubi

Degubi commented Nov 6, 2024

Copy link
Copy Markdown
Contributor Author

Not really sure about the 'Linux / msrv actix-files tests::test_head_content_length_headers' test failure, because afaict I didn't touch anything that could cause that test to fail.

About the 'public-api-diff' I saw that another PR was failing with the same error message as this PR:

This version of `cargo public-api` requires at least:
    nightly-2024-10-18
Ensure your nightly toolchain is up to date with:
    rustup install nightly --profile minimal

@robjtede robjtede changed the title Fix svg files not being compressed fix: svg files should be compressed May 10, 2025

@robjtede robjtede left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good, thanks 👍🏻

@robjtede robjtede enabled auto-merge May 10, 2025 02:24
@robjtede robjtede added this pull request to the merge queue May 10, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 10, 2025
@robjtede robjtede added this pull request to the merge queue May 10, 2025
@robjtede robjtede removed this pull request from the merge queue due to a manual request May 10, 2025
@robjtede robjtede added this pull request to the merge queue May 10, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch May 10, 2025
@robjtede robjtede enabled auto-merge May 10, 2025 03:02
@robjtede robjtede added this pull request to the merge queue May 10, 2025
Merged via the queue into actix:master with commit 3d3b31e May 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-web project: actix-web B-semver-patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants