Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixes #37464 - Support Zeitwerk loader #570

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

ofedoren
Copy link
Member

@ofedoren ofedoren commented May 15, 2024

Comment on lines 8 to 9
config.autoload_paths += Dir["#{config.root}/app/controllers/concerns"]
config.autoload_paths += Dir["#{config.root}/app/helpers/concerns"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these 2 remain, but you remove the others?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is no reason to leave them, removed.

@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch from 49e64c4 to b02cd65 Compare May 30, 2024 14:16
@ofedoren
Copy link
Member Author

Hmm, not sure why the tests from core are failing here... I had the same errors for plain Foreman, but they got fixed.

@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch from b02cd65 to cfb0a7f Compare June 22, 2024 15:37
@ofedoren ofedoren marked this pull request as ready for review June 22, 2024 16:02
@ekohl
Copy link
Member

ekohl commented Jun 24, 2024

I think this may be a relevant failure, coming from STI with interfaces.

@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch from cfb0a7f to 0155fc8 Compare June 28, 2024 15:16
@ofedoren
Copy link
Member Author

@ekohl thanks, it was indeed. Moreover, thanks to that fail we prevented a hidden issue in REX caused by the same issue.

I'm afraid there might be still places we didn't catch by the test runs :/

@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch from 0155fc8 to d23a048 Compare July 8, 2024 15:04
@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch from d23a048 to 8290692 Compare July 30, 2024 09:28
@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch 2 times, most recently from 5fdac30 to fb1725a Compare September 3, 2024 12:46
@ofedoren
Copy link
Member Author

ofedoren commented Sep 3, 2024

Ready to merge after theforeman/foreman#10131 is merged, dropped the commit for testing with Foreman PR, the latest run was green:
Screenshot_20240903_143351

@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch from fb1725a to f517cb2 Compare September 3, 2024 12:54
@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch from f517cb2 to 885082a Compare September 10, 2024 14:00
@ekohl
Copy link
Member

ekohl commented Sep 10, 2024

We'll need new releases

Because foreman_remote_execution >= 9.0, < 14 could not be found in
https://github.com/theforeman/foreman_remote_execution.git (at master@ca7cca1)
and every version of foreman_ansible depends on foreman_remote_execution >=
9.0, < 14,
  foreman_ansible cannot be used.
So, because Gemfile depends on foreman_ansible >= 0,
  version solving has failed.

@ofedoren
Copy link
Member Author

tasks and REX are done, ansible is pending

@ofedoren ofedoren force-pushed the feat-37464-support-zeitwerk branch from 885082a to d6e7cb7 Compare September 11, 2024 11:19
@ofedoren
Copy link
Member Author

🍏

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: foreman_ansible is a soft dependency so there's no version check that needs to be updated.

@ekohl ekohl merged commit 7b45829 into theforeman:master Sep 11, 2024
26 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants