-
-
Notifications
You must be signed in to change notification settings - Fork 909
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
Use autoloading instead of requiring all files up front #1317
Comments
Hi, @mcmire, I would love to work on this! |
@vsppedro Okay, that would be awesome! I will assign this to you :) |
Thanks! |
@n-rodriguez I'm not sure that Zeitwerk would be a good choice here. It seems like it's really well suited for applications, but it seems overkill for gems IMO. It would have to be added as a runtime dependency and I don't feel like people should have to bring in Zeitwerk to use shoulda-matchers. |
IMO this PR is the perfect use case for Zeitwerk since you do exactly what it does. |
@mcmire First, sorry for the problems with the PR and thank you for releasing the fix so fast. For now, I will focus on adding the tests first, as described by you here: #1333 (comment). After adding these tests I will continue with this PR - if that's okay with you. |
@vsppedro Yeah, that's fine with me! |
I'm going to close this issue. It seems like this is a relatively minor pain point and is actually more complicated than I thought it would be. |
We have a lot of files which are like active_record.rb where there are a bunch of
require
s at the top of the file. I've never liked this because there are different files at different levels being required. This is not as organized as it could be, and if you add a new file, it's confusing where to add the require. I've started usingautoload
in other projects and I find it easier to manage. We should do the same thing here.For instance, for
active_record.rb
, we might change this to something like:The text was updated successfully, but these errors were encountered: