Zeitwerk #22488
Zeitwerk #22488
Conversation
jrafanie
force-pushed
the
zeitwerk
branch
5 times, most recently
from
ac3497f to
7314322
Compare
config/application.rb
Outdated
| # config.load_defaults 6.1 | ||
| # Disable defaults as ActiveRecord::Base.belongs_to_required_by_default = true causes MiqRegion.seed to fail validation on belongs_to :maintenacne zone |
There was a problem hiding this comment.
Is this to remind us later to try to move back to defaults?
There was a problem hiding this comment.
yeah, this is still wip and has some things I want to review as we get closer.
config/initializers/zeitwerk.rb
Outdated
| # These specific directories are for code organization, not namespacing: | ||
| Rails.autoloaders.main.collapse(Rails.root.join("lib/manageiq/reporting/charting")) | ||
| Rails.autoloaders.main.collapse(Rails.root.join("lib/ansible/runner/credential")) |
There was a problem hiding this comment.
I presume we should fix these in a follow up to be properly namespaced?
There was a problem hiding this comment.
yeah, probably... I have a few things in here that will likely need followup issues.
config/initializers/zeitwerk.rb
Outdated
| /Users/joerafaniello/.gem/ruby/3.0.6/bundler/gems/manageiq-providers-nuage-d87b99b29624/lib/manageiq-providers-nuage.rb | ||
| ]) | ||
|
|
||
| # requires qpid, which isn't available on my system |
There was a problem hiding this comment.
Is there a way to handle these "optional" requires?
There was a problem hiding this comment.
Good question. I still need to figure this out.
There was a problem hiding this comment.
For now I just programmatically build a path to the file.
# requires qpid, which is an optional dependency because LOL mac
message_handler_path = Pathname.new(Vmdb::Plugins.paths[ManageIQ::Providers::Nuage::Engine]).join("app/models/manageiq/providers/nuage/network_manager/event_catcher/messaging_handler.rb")
Rails.autoloaders.main.ignore(message_handler_path)
|
Does feel like the Or if we are about ready for the big bang, then that works too |
jrafanie
force-pushed
the
zeitwerk
branch
2 times, most recently
from
7f97ba0 to
2724710
Compare
Gemfile
Outdated
| @@ -9,7 +9,7 @@ require File.join(Bundler::Plugin.index.load_paths("bundler-inject")[0], "bundle | |||
| # | |||
| # VMDB specific gems | |||
| # | |||
| gem "manageiq-gems-pending", ">0", :require => 'manageiq-gems-pending', :git => "https://github.com/ManageIQ/manageiq-gems-pending.git", :branch => "master" | |||
| gem "manageiq-gems-pending", ">0", :git => "https://github.com/jrafanie/manageiq-gems-pending.git", :branch => "zeitwerk" | |||
There was a problem hiding this comment.
TODO, remove this once gem has been updated
jrafanie
force-pushed
the
zeitwerk
branch
4 times, most recently
from
8c94c2b to
d4e2475
Compare
| @@ -6,6 +6,7 @@ | |||
| let(:missing_model) { model_directory.join("zzz_model.rb") } | |||
|
|
|||
| before do | |||
| skip "This is currently not testable with zeitwerk!" if Rails.application.config.autoloader == :zeitwerk | |||
There was a problem hiding this comment.
I tried many things to test this with zeitwerk but either can't modify the autoload paths OR, it's too late and the changes to these paths aren't used. Basically, we can't do proper setup/teardown to test this with zeitwerk. Cross repo fails in manageiq-providers-vmware if you remove the yaml autoloader, see above.
Nuage depends on qpid_proton which we can't require on mac In dev, if we eager load the application, such as with `rake zeitwerk:check`, this file blows up on mac because qpid_proton is an optional dependency but actively referenced in messaging_handler.rb. Only ignore loading qpid_proton here on mac.
Add note about rails 6.1 defaults for when we start work on rails 7: Disable defaults as ActiveRecord::Base.belongs_to_required_by_default = true causes MiqRegion.seed to fail validation on belongs_to :maintenacne zone.
Fixes the following with zeitwerk when running the spec/lib/pdf_generator_spec.rb test: expected file /Users/joerafaniello/Code/manageiq/lib/pdf_generator/null_pdf_generator.rb to define constant PdfGenerator::NullPdfGenerator, but didn't
We can't modify the frozen autoload paths with zeitwerk. For now, just disable this test with zeitwerk loader.
It's far easier to debug in dev/test/containers using an ENV variable so we'll make it so zeitwerk stdout logging and eager loading of all engines is only done with this flag enabled.
|
Some comments on commits jrafanie/manageiq@17a20e8~...738b833 config/initializers/zeitwerk.rb
|
|
Checked commits jrafanie/manageiq@17a20e8~...738b833 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
|
Skipping backport to |
Depends on:
This PR enabled zeitwerk. It adds some acronyms/inflections that are backward compatible but required for zeitwerk. Most of the extractable content has been extracted in other PRs.
Regarding the debug flag
DEBUG_MANAGEIQ_ZEITWERK:We have a debug flag to enable logging and testing of the plugin autoload paths by eager loading them:
With no debug flag, we get not debug output and engines autoload paths are not eager loaded:
With debugging, we get debug logging and we eager load all autoload paths to ensure those plugins define things correctly: