KTOR-9355 Deprecate HttpHeaders.AcceptCharset#5411
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (9)
WalkthroughThis PR deprecates the Accept-Charset HTTP header constant across the Ktor framework and adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.request.acceptCharsetItems) | ||
| */ | ||
| @Suppress("DEPRECATION") | ||
| public fun ApplicationRequest.acceptCharsetItems(): List<HeaderValue> = |
There was a problem hiding this comment.
Should we also deprecate these functions? My thought was that we should discourage users from adding this header, but it's okay to try to read it, because server is usually out of control.
There was a problem hiding this comment.
I think reading it in is fine, maybe exclude this one.
bjhham
left a comment
There was a problem hiding this comment.
Could you include some KDoc or a deprecation message that either references or summarizes the reason in the RFC note?
| * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.request.acceptCharsetItems) | ||
| */ | ||
| @Suppress("DEPRECATION") | ||
| public fun ApplicationRequest.acceptCharsetItems(): List<HeaderValue> = |
There was a problem hiding this comment.
I think reading it in is fine, maybe exclude this one.
| @Deprecated( | ||
| "The Accept-Charset request header has been deprecated in RFC 9110. " + | ||
| "UTF-8 should be used by default in modern applications." | ||
| ) | ||
| public const val AcceptCharset: String = "Accept-Charset" |
There was a problem hiding this comment.
@bjhham, could you check if this deprecation message is enough?
Subsystem
ktor-httpMotivation
KTOR-9355 Deprecate HttpHeaders.AcceptCharset