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.

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

remove BodyEncoding trait #2565

Merged
merged 5 commits into from
Jan 3, 2022
Merged

remove BodyEncoding trait #2565

merged 5 commits into from
Jan 3, 2022

Conversation

robjtede
Copy link
Member

@robjtede robjtede commented Jan 3, 2022

PR Type

Refactor

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

The BodyEncoding trait is largely unnecessary.

This changes encoding signalling to only use the Content-Encoding header which was necessary to use anyway to stay spec compliant. The Compress middleware already skips encoding for any responses with this header so the sole function of this trait was to force an encoding.

Therefore, the only specific function this trait had was to force an encoding on a per-response basis; I consider this too niche a use case to support.

@robjtede robjtede added A-web project: actix-web B-semver-major breaking change requiring a major version bump labels Jan 3, 2022
@robjtede robjtede added this to the actix-web v4 milestone Jan 3, 2022
@robjtede robjtede marked this pull request as ready for review January 3, 2022 18:03
@robjtede robjtede changed the title remove BodyEncoding trait remove BodyEncoding trait Jan 3, 2022
@robjtede robjtede merged commit 0bc4ae9 into master Jan 3, 2022
@robjtede robjtede deleted the remove-body-encoding branch January 3, 2022 18:46
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-major breaking change requiring a major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant