-
Notifications
You must be signed in to change notification settings - Fork 8
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
Compatibility with Zeitwerk #27
Conversation
a8bbb1f
to
446df67
Compare
@ofedoren I couldn't quite figure this out. I had a theory that DHCP was an acronym that broke it, but that doesn't appear to work. Could you have a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @ekohl, pretty much everything is covered as far as I can tell, nice one 🥇
It seems the plugin is even older than me, it lacks inline suggestion at least:
@@ -1,16 +1,19 @@ | |||
require 'deface' | |||
module ForemanDhcpBrowser | |||
module ForemanDHCPBrowser | |||
class Engine < ::Rails::Engine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class Engine < ::Rails::Engine | |
class Engine < ::Rails::Engine | |
engine_name 'foreman_dhcp_browser' |
Nice. Now it fails because of:
This is because there is no such task because it doesn't have tests. I'll modify CI to skip it so we just rely on seeds and assets (which already identified if the plugin could be loaded or not). |
Since Ruby 2.1 this is allowed and we can rely on that.
With a dummy task this is now green. |
Much appreciated! How can I apply this update, so I can test drive it? |
I've submitted #28 so that for every PR we create an RPM you can test. No guarantee it works on a stable branch, but should work on nightly. I should create a release, but I got a bit distracted with investigating the best way to do so. |
Looks like
|
Yes, but we dropped EL7 and I don't think we'll ever introduce it again so we might as well clean up the spec |
My foreman is broken at the moment. I need it working for an urgent update in the company. EDIT: puppet is working again, theforman is dead. |
I just released 0.1.1 (0.1.0 broke because of an issue in the release workflow) so I'll look at packaging as well. |
theforeman/foreman-packaging#11657 & theforeman/foreman-packaging#11654 are for nightly. |
Completely untested by now, which is why I added CI.
Fixes #26