-
-
Notifications
You must be signed in to change notification settings - Fork 160
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 quote parsing from package #180
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
20eab92
to
3357666
Compare
@gurgunday I think if we're expecting other applications (re #179 (comment)) it might be an issue merging this. Since another application might be using quotes. I'm still of the opinion that it's fine to shift this responsibility into |
Yeah, just checked the RFC again, nothing about this quote removal behavior So I agree then that this should be a decoder responsibility, and since the default encoder doesn't do quotes, I don't see any reason to expect or even try to unquote values |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #180 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 159 152 -7
Branches 67 65 -2
=========================================
- Hits 159 152 -7 ☔ View full report in Codecov by Sentry. |
Another RFC (cc @gurgunday). Removes a single check that wouldn't be relevant if this package set the cookie to begin with. Clients do not surreptitiously add quotes to cookies (as far as I know?), it's just a cookie value is valid to be surrounded by quotes.