Skip to content
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

Fixes #37471 - Support Zeitwerk loader #10997

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ofedoren
Copy link
Contributor

WIP

What are the changes introduced in this pull request?

This PR should make Katello plugin compatible with Zeitwerk autoloader, which will be enabled in Foreman instead of classic Rails' loader.

Considerations taken when implementing this change?

I was trying to change rather less than more. Although, some stuff (like CV acronym) can be introduced, but will require more changes and I'm not sure it'll be quite safe. It's up to maintainers.

What are the testing steps for this pull request?

Test with Foreman main PR (included in CI workflow) and hope for the best. Note: at first it'll be red, since some plugins, which are dependencies, should be updated as well. Additionally, after the tests are green I strongly recommend to test out basic/advanced content related workflows live.

@jeremylenz
Copy link
Member

jeremylenz commented May 23, 2024

I had to check out:

I tested

  • refreshing my manifest
  • deleting a subscription from my manifest
  • enabling a RH repository
  • disabling a RH repository
  • publish and promote a CV
  • remove a package from a custom repository
  • create new blank docker repo
  • syncing all repos

Seems all good so far! I grepped development.log for zeitwerk and "uninitialized constant" and didn't see anything concerning.

@jeremylenz
Copy link
Member

also notable: With the foreman-tasks PR checked out, I did not see this error

Zeitwerk::NameError: expected file /home/runner/work/katello/katello/vendor/bundle/ruby/2.7.0/gems/foreman-tasks-9.1.1/app/lib/actions/middleware/keep_current_request_id.rb to define constant Actions::Middleware::KeepCurrentRequestId, but didn't

which is currently failing in CI.

@ofedoren ofedoren force-pushed the feat-37471-support-zeitwerk branch from 8859c89 to 0c72da8 Compare May 30, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants