-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Reduce code indent in ResponseHandler.data_received #8699
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #8699 +/- ##
==========================================
+ Coverage 98.73% 98.75% +0.01%
==========================================
Files 121 121
Lines 36750 36789 +39
Branches 4391 4395 +4
==========================================
+ Hits 36286 36330 +44
+ Misses 314 312 -2
+ Partials 150 147 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Looks like we are missing coverage for |
I feel like that relates to #8597 (comment) or one of my other parser changes. I'm not really sure if it's still possible to reach that code at all... |
I'll dig through the code paths this weekend if flights cooperate. |
Looks like websocket will never have a tail unless there is an exception |
I spent a few hours digging at it, and I can't find a case or make a case where |
Then I reckon remove it, but also skip the 3.10 backport as a precaution. |
be5c09f
to
d586152
Compare
Still having trouble getting comfortable with removing that code, need to spend a few more hours tracing |
d586152
to
896a2f8
Compare
I wonder if it is possible and we don't have coverage for it. We didn't have test coverage for split WebSocket payloads until #9649 |
CodSpeed Performance ReportMerging #8699 will not alter performanceComparing Summary
|
896a2f8
to
f1f7b72
Compare
After discovering aiohttp/aiohttp/_http_parser.pyx Line 574 in 2e369db
|
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 11f0e7f on top of patchback/backports/3.11/11f0e7f0a9be7ab27036a49ff13e318ed03b769a/pr-8699 Backporting merged PR #8699 into master
🤖 @patchback |
Backport to 3.12: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 11f0e7f on top of patchback/backports/3.12/11f0e7f0a9be7ab27036a49ff13e318ed03b769a/pr-8699 Backporting merged PR #8699 into master
🤖 @patchback |
(cherry picked from commit 11f0e7f)
(cherry picked from commit 11f0e7f)
What do these changes do?
Code cleanup only, no functional change
There were some else after returns that could be converted to guards to reduce code indent.
While looking at the full WebSockets path for #8258 I noticed this could be improved a bit
Additionally, some missing test coverage is added and unreachable code is removed.
Are there changes in behavior for the user?
no
Is it a substantial burden for the maintainers to support this?
no