Make rails engines zeitwerk autoloader friendly #205
Conversation
These aren't widely known acronyms, so Zeitwerk and rails need to be told about them. We should avoid using acronyms and default to using the "rails" convention unless it's warranted to specify acronyms. All other usages of the same acronyms would need to be enforced in locations so it's not without side effects. It's just not worth it here.
There is no need create a file called lib/my-gem.rb. Bundler will automatically treat this as a namespace due to the hyphen, so it will try a my-gem require first and then fallback on my/gem. See: https://github.com/rubygems/rubygems/blob/a223274e4627e8a76112415c0e38f8a45a42f97e/bundler/lib/bundler/runtime.rb#L72-L73 Because we're moving towards zeitwerk, ruby files should be named based on their namespace/class names. 1) Moving / renaming based on a namespaced gem name works automatically with bundler. 2) Zeitwerk knows to ignore this file, so we don't need to.
|
cc @agrare |
|
Asid, but semi-related, do we even "use" this in the core app - we've wanted to remove this repo for a while, and theres a small subset that's actually used that we should move to the core repo. |
yup, I can see it used here: app/models/chargeback.rb: plan = ManageIQ::Showback::PricePlan.find_or_create_by(:description => rate.description,
app/models/chargeback_rate_detail.rb: showback_rate = ManageIQ::Showback::Rate.find_or_create_by(:entity => entity,
spec/models/chargeback_configured_system_spec.rb: ManageIQ::Showback::InputMeasure.seed
spec/models/chargeback_container_project_spec.rb: ManageIQ::Showback::InputMeasure.seed
spec/models/chargeback_vm_spec.rb: ManageIQ::Showback::InputMeasure.seedI'd love to remove it or the unused parts but I'm just not very confident in how it works to do it. |
|
Note, if we go the route of eager loading with zeitwerk, there are two additional changes we can do later on: diff --git a/lib/manageiq/consumption.rb b/lib/manageiq/consumption.rb
index b0823f2..3e0a410 100644
--- a/lib/manageiq/consumption.rb
+++ b/lib/manageiq/consumption.rb
@@ -1,2 +1,5 @@
require "manageiq/showback/engine"
require "manageiq/showback/version"
+
+module ManageIQ::Consumption
+end
diff --git a/lib/manageiq/showback/engine.rb b/lib/manageiq/showback/engine.rb
index 34dcbbb..dd804ef 100644
--- a/lib/manageiq/showback/engine.rb
+++ b/lib/manageiq/showback/engine.rb
@@ -6,6 +6,7 @@ module ManageIQ
isolate_namespace ManageIQ::Showback
config.autoload_paths << root.join('lib').to_s
+ config.eager_load_paths << root.join('lib').to_s
def self.vmdb_plugin?
true |
|
Checked commits jrafanie/manageiq-consumption@702dd87~...d883ad5 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
|
This is ready for review/merge. |
|
fun: |
lib/manageiq/consumption.rbinstead oflib/manageiq-consumption.rbas the former is zeitwerk friendly and both are acceptable for bundlerNote, this is an attempt to minimize the changes in the engine but more changes may be needed when we drop the old loader and switch to zeitwerk. This change should be mergeable today without changing to zeitwerk and is an example of what I'll be doing in other engines.