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

Allow Rack::Builder#run to take a block instead of an argument. #1942

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

ioquatix
Copy link
Member

@ioquatix ioquatix commented Aug 4, 2022

When I was first introduced to rack many years ago, I recalled that this particular part of the config.ru caused me a lot of initial confusion.

I think we should allow run {|env| ...} to work, it's more canonical Ruby and it's easy for new users to understand since they don't need to learn about multiple concepts (e.g. lambdas).

I can't see any major downsides to accepting this, except it will mean older servers won't be compatible with the new syntax, but I think that's an acceptable trade off given how many other things are also changing.

@ioquatix ioquatix self-assigned this Aug 4, 2022
@ioquatix ioquatix added this to the v3.0.0 milestone Aug 4, 2022
@ioquatix ioquatix force-pushed the rack-builder-run-block branch from cc4192c to 9fd4936 Compare August 4, 2022 05:07
Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

One small change requested. Otherwise looks fine.

lib/rack/builder.rb Show resolved Hide resolved
@ioquatix ioquatix requested a review from jeremyevans August 4, 2022 06:39
@ioquatix ioquatix enabled auto-merge (squash) August 4, 2022 06:40
@ioquatix ioquatix mentioned this pull request Aug 4, 2022
@ioquatix ioquatix merged commit 1e4c18d into main Aug 4, 2022
@ioquatix ioquatix deleted the rack-builder-run-block branch August 4, 2022 14:36
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.

3 participants