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

fix: describe module autoloading in config.ru #1976

Merged
merged 2 commits into from
Oct 21, 2022
Merged

Conversation

yumetodo
Copy link
Contributor

It seems to me that described at #1975 is not well documented.

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.

Note that the same behavior was true in Rack 2 (need to require 'rack' to get autoloading), it just wasn't necessarily safe in Rack 2 to require specific components, since the components didn't specify their dependencies (nor were there tests for requiring specific components). I think that the difference is that the extracted rackup gem only loads the parts of rack that it needs, while in Rack 2, rackup does require 'rack'.

If you are using a separate server (puma/unicorn/falcon/etc.), I don't think there is a difference in behavior. As that is the vast majority of production usage, this documentation should be specific that it only applies to rackup, and moved to the section that discusses rackup.

@yumetodo
Copy link
Contributor Author

@jeremyevans Thank you for your review. To follow your review, I move module autoloading section into rackup section.

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.

Looks good, thanks!

@ioquatix ioquatix merged commit 0a62f75 into rack:main Oct 21, 2022
@ioquatix
Copy link
Member

This is really great, thanks for pointing out the issue here and making a PR to improve the documentation.

@yumetodo yumetodo deleted the patch-1 branch October 22, 2022 03:58
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