Skip to content

Conversation

cee-dub
Copy link
Collaborator

@cee-dub cee-dub commented Sep 16, 2025

Follow up to #186, responding to comments.

@cee-dub cee-dub requested review from joeshaw and dgryski September 16, 2025 00:46
@cee-dub
Copy link
Collaborator Author

cee-dub commented Sep 16, 2025

It appears we need to update another integration test since underlyingReaderFrom() can detect the reader inside an io.NopCloser.

Copy link
Member

@joeshaw joeshaw left a comment

Choose a reason for hiding this comment

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

👍

@joeshaw
Copy link
Member

joeshaw commented Sep 16, 2025

It appears we need to update another integration test since underlyingReaderFrom() can detect the reader inside an io.NopCloser.

Can you say more? When I last looked it was not possible to see inside of io.NopCloser, which is why we have our own implementation of it in fsthttp which can.

@cee-dub
Copy link
Collaborator Author

cee-dub commented Sep 16, 2025

Can you say more? When I last looked it was not possible to see inside of io.NopCloser, which is why we have our own implementation of it in fsthttp which can.

I may have misread the code in seeking an explanation for the repeated failure (flake?) of the io.NopCloser test that's currently failing in CI.

@cee-dub
Copy link
Collaborator Author

cee-dub commented Sep 16, 2025

Oh, it is a flake! My GitHub app UI was stale and showed a failure from yesterday. It passed after the latest re-run.

@cee-dub cee-dub merged commit 72dbc29 into main Sep 16, 2025
10 of 14 checks passed
@cee-dub cee-dub deleted the cee-dub/limit-docs branch September 16, 2025 15:12
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.

2 participants