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

Rename UNPROCESSABLE_ENTITY to UNPROCESSABLE_CONTENT #692

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Pi-Cla
Copy link
Contributor

@Pi-Cla Pi-Cla commented Mar 20, 2024

Fix #664

@seanmonstar
Copy link
Member

I believe this would be a breaking change, since an existing constant would no longer exist.

@obi1kenobi 👋 curious, is this something that the semver job should be able to notice?

@Pi-Cla
Copy link
Contributor Author

Pi-Cla commented Mar 20, 2024

Is there any way to have there be both a StatusCode:UNPROCESSABLE_ENTITY and a StatusCode:UNPROCESSABLE_CONTENT which point to the same error number?
@seanmonstar

I tried to just copy the tuple so that now it looked like this:

/// 422 Unprocessable Content
/// [[RFC9110, Section 15.5.21](https://datatracker.ietf.org/doc/html/rfc9110#section-15.5.21)]
(422, UNPROCESSABLE_CONTENT, "Unprocessable Entity");
/// 422 Unprocessable Content
/// [[RFC9110, Section 15.5.21](https://datatracker.ietf.org/doc/html/rfc9110#section-15.5.21)]
(422, UNPROCESSABLE_ENTITY, "Unprocessable Entity");

but that resulted in an unreachable_patterns error up in canonical_reason

@seanmonstar
Copy link
Member

I think another PR exists which modifies the macro to allow "aliases".

@obi1kenobi
Copy link

I believe this would be a breaking change, since an existing constant would no longer exist.

@obi1kenobi 👋 curious, is this something that the semver job should be able to notice?

Thanks for the ping, I appreciate it!

We currently have the equivalent lint for free (module-level) constants, but don't have the lint for constants inside a type's impl block yet: obi1kenobi/cargo-semver-checks#366 (comment)

It's very easy to add, and we have a few contributors who are actively adding lints right now, so we can prioritize it without any trouble. It'll be in the next release for sure.

@seanmonstar
Copy link
Member

Ya no worries, I know it's neverending work. Thanks so much!

@obi1kenobi
Copy link

Thank you for using it, and for the supportive feedback! It really helps me prioritize things.

If there are other things that would be useful, I'd love to hear about them! Either a GitHub issue or a 15min chat (and I can file issues as needed) would work. Ultimately a lot of things are funding-limited, but it helps me know what to say users want when I apply for OSS grants!

@obi1kenobi
Copy link

Just merged the lint that will catch issues like this going forward: obi1kenobi/cargo-semver-checks#714

It'll be part of the next cargo-semver-checks release, planned for a day two after Rust 1.78 so ~first week of May.

One fewer breaking change that can sneak unnoticed into our crates!

@obi1kenobi
Copy link

I ended up cutting a new release earlier than originally planned: https://github.com/obi1kenobi/cargo-semver-checks/releases/tag/v0.31.0

The new inherent_associated_pub_const_missing lint will catch cases like this going forward. Thanks again for your patience and support, and for maintaining so many awesome crates! ❤️

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.

Rename 422 from "Unprocessable Entity" to "Unprocessable Content"
3 participants