Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Config doesn't have to be a Rails Engine #247

Merged
merged 4 commits into from
Jan 3, 2020

Conversation

amatsuda
Copy link
Member

I found that Config defines an isolated Engine class under Rails environment, but this simple plugin actually does not use Rails Engine feature at all. This plugin already has a Railtie, and that's enough for doing things that this plugin offers.

Creating an unused Engine class like this is not just unnecessary but indeed rather harmful for the users' applications, because it slows down the application initialization process, consumes much memory, and slows down auto/eager loading, request handling, rendering, etc. by adding entries for them.

@pkuczynski
Copy link
Member

@amatsuda thank you for your contribution. Could you please add CHANGELOG entry?

@pyromaniac @rdubya @supremebeing7 what you think about this PR?

@amatsuda
Copy link
Member Author

@pkuczynski

Could you please add CHANGELOG entry?

Done!

supremebeing7
supremebeing7 previously approved these changes Aug 16, 2019
Copy link
Contributor

@supremebeing7 supremebeing7 left a comment

Choose a reason for hiding this comment

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

👍

rdubya
rdubya previously approved these changes Aug 17, 2019
Copy link
Contributor

@rdubya rdubya left a comment

Choose a reason for hiding this comment

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

Looks ok to me

@pkuczynski pkuczynski dismissed stale reviews from rdubya and supremebeing7 via f856a29 January 3, 2020 18:39
@pkuczynski pkuczynski added this to the 2.2.0 milestone Jan 3, 2020
@pkuczynski pkuczynski merged commit ceeba6d into rubyconfig:master Jan 3, 2020
@pkuczynski
Copy link
Member

Thanks! Sorry for such a long time for merging this one...

@pkuczynski
Copy link
Member

Will be released soon with 2.2.0

@amatsuda amatsuda deleted the unengine branch January 3, 2020 19:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging this pull request may close these issues.

4 participants