Skip to content

Fix respond_to?(:each) with Rack::Lint with streaming bodies. - #1981

Merged
ioquatix merged 4 commits into
mainfrom
rack-lint-respond_to
Nov 11, 2022
Merged

ioquatix merged 4 commits into
mainfrom
rack-lint-respond_to

Conversation

@ioquatix

@ioquatix ioquatix commented Nov 11, 2022 •

Copy link
Copy Markdown
Member

This is the last piece required to get webrick passing the conformance test suite with Rack 3.

@ioquatix
ioquatix requested a review from jeremyevans November 11, 2022 08:25
@ioquatix
ioquatix enabled auto-merge (squash) November 11, 2022 08:26
@ioquatix

Copy link
Copy Markdown
Member Author

Confirmed fix in socketry/rack-conform#11

@ioquatix
ioquatix disabled auto-merge November 11, 2022 09:10

@jeremyevans jeremyevans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

to_ary and each handling looks correct. I can't see where to_path is implemented (there is verify_to_path, called by each). If you are going to add this, I think you should implement Wrapper#to_path, and have it call @body.to_path. Otherwise, if the body responds to to_path, Wrapper will say it responds to to_path when it doesn't, assuming I'm reading the code correctly.

@ioquatix
ioquatix requested a review from jeremyevans November 11, 2022 20:09
@ioquatix
ioquatix merged commit 00aa541 into main Nov 11, 2022
@ioquatix
ioquatix deleted the rack-lint-respond_to branch November 11, 2022 23:37
ioquatix added a commit that referenced this pull request Nov 18, 2022
)

* Fix handling of `to_path` and add spec.

* Also forward `call`.
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