Fix compatibility with Zeitwerk#160
Conversation
ofedoren
left a comment
ofedoren
left a comment
There was a problem hiding this comment.
Thanks, @ekohl, I thought this plugin is safe, since #158 didn't show any errors.
Now, just to double check, could you please add a commit to test with theforeman/foreman#10299?
lib/foreman_kubevirt/engine.rb
Outdated
| @@ -4,7 +4,7 @@ class Engine < ::Rails::Engine | |||
| config.autoload_paths += Dir["#{config.root}/app/models/concerns"] | |||
| initializer "foreman_kubevirt.register_plugin", :before => :finisher_hook do |_app| | |||
| Foreman::Plugin.register :foreman_kubevirt do | |||
There was a problem hiding this comment.
Just to be aligned with other plugins, we could probably wrap this in app.reloader.to_prepare block.
There was a problem hiding this comment.
Perhaps that also solves the deprecation warning I still saw. Will try
There was a problem hiding this comment.
Could you submit that as a change in https://github.com/theforeman/foreman_plugin_template/blob/master/lib/foreman_plugin_template/engine.rb as well?
There was a problem hiding this comment.
Perhaps that also solves the deprecation warning I still saw. Will try
That it does.
There was a problem hiding this comment.
Could you submit that as a change in https://github.com/theforeman/foreman_plugin_template/blob/master/lib/foreman_plugin_template/engine.rb as well?
|
It's green both with and without: |
|
Waiting for #165, marking as draft to be sure. |
|
@ofedoren does it look good to you? |
ofedoren
left a comment
ofedoren
left a comment
There was a problem hiding this comment.
Overall LGTM, but just to be more nitpicky:
lib/foreman_kubevirt/engine.rb
Outdated
| @@ -2,18 +2,20 @@ module ForemanKubevirt | |||
| class Engine < ::Rails::Engine | |||
| engine_name "foreman_kubevirt" | |||
| config.autoload_paths += Dir["#{config.root}/app/models/concerns"] | |||
There was a problem hiding this comment.
I'd drop this. If an error occurs then we should fix it (there might be uninitialized constant error or something similar). Such paths are already come from Foreman/Rails themselves.
ekohl
force-pushed
the
rails-7-compat
branch
from
236e913 to
36cd448
Compare
4 participants
No description provided.