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

Replace public use of mime crate with &str #642

Merged
merged 1 commit into from
Dec 21, 2021

Conversation

nashley
Copy link
Contributor

@nashley nashley commented Dec 21, 2021

This PR introduces a breaking change.

Motivation

Resolves #637.
Removes exposure of the mime crate in the public library.
This will allow users to handle the underlying &str instead and optionally parse Content-Type headers with their library of choice (if desired).
This also allows axum to remove the dependency on mime in the future without requiring another breaking change.

Solution

&str has replaced &mime::Mime as axum::extract::multipart::Field::content_type's return type

Searching docs for mime shows no results:

> cd axum/src/docs
> rg mime
> # (no results)

Questions

  • Does this PR need to be against a different branch other than main for the 0.5 release?
  • Should there be more tests? I couldn't find a good way to create parameterized tests without adding another dependency or creating a potentially-esoteric macro.
  • Should there be a doc test for content_type()? Seems like it'd either be excessive or incomplete.

Replaces `Field::content_type`'s return type with `&str`.
This is a breaking change.

Closes tokio-rs#637
@davidpdrsn davidpdrsn added this to the 0.5 milestone Dec 21, 2021
@davidpdrsn davidpdrsn added A-axum C-enhancement Category: A PR with an enhancement breaking change A PR that makes a breaking change. labels Dec 21, 2021
@davidpdrsn
Copy link
Member

Does this PR need to be against a different branch other than main for the 0.5 release?

I'm gonna make a branch for 0.5 soon and then we can merge this into that.

Should there be more tests? I couldn't find a good way to create parameterized tests without adding another dependency or creating a potentially-esoteric macro.

Thank you for adding a test! I dont think we need to add more tests here 😅

Should there be a doc test for content_type()? Seems like it'd either be excessive or incomplete.

Nah I think it's fine as is.

@davidpdrsn davidpdrsn changed the base branch from main to axum-next December 21, 2021 09:47
@davidpdrsn davidpdrsn merged commit 25ff831 into tokio-rs:axum-next Dec 21, 2021
@nashley nashley deleted the remove-mime-from-public-api branch December 21, 2021 17:08
davidpdrsn pushed a commit that referenced this pull request Dec 27, 2021
Replaces `Field::content_type`'s return type with `&str`.
This is a breaking change.

Closes #637
davidpdrsn pushed a commit that referenced this pull request Jan 3, 2022
Replaces `Field::content_type`'s return type with `&str`.
This is a breaking change.

Closes #637
davidpdrsn pushed a commit that referenced this pull request Jan 3, 2022
Replaces `Field::content_type`'s return type with `&str`.
This is a breaking change.

Closes #637
davidpdrsn pushed a commit that referenced this pull request Jan 23, 2022
Replaces `Field::content_type`'s return type with `&str`.
This is a breaking change.

Closes #637
davidpdrsn pushed a commit that referenced this pull request Jan 23, 2022
Replaces `Field::content_type`'s return type with `&str`.
This is a breaking change.

Closes #637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-axum breaking change A PR that makes a breaking change. C-enhancement Category: A PR with an enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change return type of Field::content_type
2 participants