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

Support default_host in Rack::Test::Methods #318

Merged
merged 1 commit into from
Aug 11, 2022

Conversation

jeremyevans
Copy link
Contributor

This allows you to configure a default host the same way you
configure the application.

Fixes #317

This allows you to configure a default host the same way you
configure the application.

Fixes rack#317
Copy link
Member

@ioquatix ioquatix left a comment

Choose a reason for hiding this comment

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

LGTM.

@ioquatix
Copy link
Member

Do you think there will be any other options in the future we want to support? I wonder whether default_host is too generic.

It might be a nice idea to have something like session_options which is a hash which gets passed to Session.new(app, **session_options) which is a little more generic and future proofed.

@jeremyevans
Copy link
Contributor Author

Currently, Session.new only accepts app and default_host. I don't have any plans to change this API. If we do decide to switch to an options hash approach, it will be fairly easy to continue to support this in a backwards compatible manner.

@jeremyevans jeremyevans merged commit 3626161 into rack:main Aug 11, 2022
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.

None yet

2 participants