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

Small RequestHandler refactor #10326

Merged
merged 5 commits into from
Jan 8, 2024
Merged

Small RequestHandler refactor #10326

merged 5 commits into from
Jan 8, 2024

Conversation

yawkat
Copy link
Member

@yawkat yawkat commented Jan 4, 2024

This PR replaces the StreamedHttpRequest passed to RequestHandler by a separate ByteBody argument. This removes the need for some instanceof checks.

This is a small part of #10131, but it makes for annoying merge conflicts, so I want to pull it into 4.3.x. It should have no functional or performance difference.

This PR replaces the StreamedHttpRequest passed to RequestHandler by a separate ByteBody argument. This removes the need for some instanceof checks.

This is a small part of #10131, but it makes for annoying merge conflicts, so I want to pull it into 4.3.x. It should have no functional or performance difference.
@yawkat yawkat added the type: improvement A minor improvement to an existing feature label Jan 4, 2024
@yawkat yawkat added this to the 4.3.0 milestone Jan 4, 2024
Copy link
Contributor

@timyates timyates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@timyates timyates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that's just a test...

@yawkat
Copy link
Member Author

yawkat commented Jan 5, 2024

huh, i didnt know it had external deps. but since it's just a test we can fix it in session.

Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some sonar issues. Could you check them @yawkat ?

@yawkat
Copy link
Member Author

yawkat commented Jan 8, 2024

they arent in the diff

@timyates
Copy link
Contributor

timyates commented Jan 8, 2024

they arent in the diff

This one is 😜

@yawkat
Copy link
Member Author

yawkat commented Jan 8, 2024

ive marked it as wontfix

@yawkat yawkat force-pushed the no-streaming-request branch from 4f471f3 to 5746ff2 Compare January 8, 2024 10:03
Copy link

sonarqubecloud bot commented Jan 8, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

4 New issues
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@yawkat yawkat merged commit b279708 into 4.3.x Jan 8, 2024
@yawkat yawkat deleted the no-streaming-request branch January 8, 2024 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A minor improvement to an existing feature
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants