Introduce lazy loading as an optional config#3228
Introduce lazy loading as an optional config#3228stefannibrasil merged 6 commits intolazy-loading-betafrom
Conversation
stefannibrasil
force-pushed
the
sb/introduce-lazy-load
branch
3 times, most recently
from
103500c to
0c9dc19
Compare
To keep this change backwards compatible, we introduce lazy loading as a config option. We need to load main classes first for nested generators such as 'music' and when lazy loading is not enabled. 'internet/http'. This will avoid the need for renaming these generators. Co-authored-by: Thiago Araujo <thd.araujo@gmail.com>
stefannibrasil
force-pushed
the
sb/introduce-lazy-load
branch
from
d4e9ddb to
4c06713
Compare
stefannibrasil
changed the title
stefannibrasil
force-pushed
the
sb/introduce-lazy-load
branch
from
4c06713 to
1ca07ae
Compare
|
|
||
| - name: Run tests | ||
| run: bundle exec rake test | ||
| test-with-lazy-loading: |
There was a problem hiding this comment.
I don't think we need to run the tests for every ruby version. Just one is enough?
| end | ||
| end | ||
|
|
||
| if Faker::Config.lazy_loading? |
There was a problem hiding this comment.
could this be a mixin, similar to https://github.com/rails/rails/blob/main/activesupport/lib/active_support/dependencies/autoload.rb#L29?
for example:
if Faker::Config.lazy_loading?
extend Faker::Autoload
lazy_load(self)
end
Or maybe the loader takes care of checking the flag and everything...
extend Faker::Autoload
lazy_load(self)
end
I wish we could at least namespace this stuff to something like Faker::Autoload or something. Not a requirement for the beta, though!
There was a problem hiding this comment.
Thank you, that's a good idea. I will leave that for after. I want to get this beta release out first 🤞🏼
|
|
||
| def lazy_loading? | ||
| Thread.current[:faker_lazy_loading] == true | ||
| if ENV.key?('FAKER_LAZY_LOAD') && !ENV['FAKER_LAZY_LOAD'].nil? |
There was a problem hiding this comment.
nit: maybe we should also handle TRUE and true values?
Covers the case of users setting the env using truthy values.
2 participants
Motivation / Background
Closes #3161
This Pull Request has been created because after running experiments documented in https://github.com/faker-ruby/faker/tree/main/experiments, we decided to go with lazy loading, and we want to release a beta version for users to report any issues using lazy loading.
Lazy loading is disabled by default and can be enabled by setting a config:
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]