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

There can be only one autoloader: zeitwerk #22801

Merged

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Dec 4, 2023

Part of the rails 7 upgrade: #22052

Should be merged with:

Cross repo test:

  • Drop test not testable with zeitwerk. We can't dynamically modify the autoload_paths from within the test when zeitwerk is the autoloader.
  • Drop as_dependencies_interlock as zeiterk doesn't need it.
  • Drop conditional as zeitwerk is the only supported autoloader

@jrafanie jrafanie requested a review from Fryguy as a code owner December 4, 2023 21:00
@miq-bot miq-bot added the wip label Dec 4, 2023
@jrafanie jrafanie changed the title [WIP] There can be only one autoloader zeitwerk [WIP] There can be only one autoloader: zeitwerk Dec 4, 2023
@miq-bot miq-bot added the wip label Dec 4, 2023
jrafanie added a commit to jrafanie/manageiq-automation_engine that referenced this pull request Dec 4, 2023
This was only needed for classic autoloader.  The core "freedom" patch made zeitwerk autoloader
bypass the interlock anyway, so now that we're only supporting zeitwerk, this is no longer
needed.

Co-dependency:
ManageIQ/manageiq#22801

Part of the rails 7 upgrade: ManageIQ/manageiq#22052
jrafanie added a commit to jrafanie/manageiq-api that referenced this pull request Dec 4, 2023
This was only needed for classic autoloader.  The core "freedom" patch made zeitwerk autoloader
bypass the interlock anyway, so now that we're only supporting zeitwerk, this is no longer
needed.

Co-dependency:
ManageIQ/manageiq#22801

Part of the rails 7 upgrade: ManageIQ/manageiq#22052
jrafanie added a commit to jrafanie/manageiq-api that referenced this pull request Dec 4, 2023
This was only needed for classic autoloader.  The core "freedom" patch made zeitwerk autoloader
bypass the interlock anyway, so now that we're only supporting zeitwerk, this is no longer
needed.

Co-dependency:
ManageIQ/manageiq#22801

Part of the rails 7 upgrade: ManageIQ/manageiq#22052
@jrafanie
Copy link
Member Author

jrafanie commented Dec 4, 2023

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Dec 4, 2023
@jrafanie jrafanie changed the title [WIP] There can be only one autoloader: zeitwerk There can be only one autoloader: zeitwerk Dec 4, 2023
@jrafanie jrafanie removed the wip label Dec 4, 2023
jrafanie added a commit to jrafanie/manageiq-automation_engine that referenced this pull request Dec 18, 2023
This was only needed for classic autoloader.  The core "freedom" patch made zeitwerk autoloader
bypass the interlock anyway, so now that we're only supporting zeitwerk, this is no longer
needed.

Co-dependency:
ManageIQ/manageiq#22801

Part of the rails 7 upgrade: ManageIQ/manageiq#22052
We can't dynamically modify the autoload_paths from within the test when
zeitwerk is the autoloader.

Part of the rails 7 upgrade:
ManageIQ#22052
@jrafanie jrafanie force-pushed the there_can_be_only_one_autoloader_zeitwerk branch from 803e368 to fb5f6e8 Compare December 18, 2023 20:57
@miq-bot
Copy link
Member

miq-bot commented Dec 18, 2023

Some comments on commits jrafanie/manageiq@333ab09~...fb5f6e8

config/initializers/zeitwerk.rb

  • ⚠️ - 2 - Detected puts. Remove all debugging statements.

@miq-bot
Copy link
Member

miq-bot commented Dec 18, 2023

Checked commits jrafanie/manageiq@333ab09~...fb5f6e8 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@jrafanie
Copy link
Member Author

This is ready to go since we were already defaulting to zeitwerk on quinteros

@Fryguy Fryguy merged commit 7a3c4a6 into ManageIQ:master Dec 19, 2023
6 checks passed
@Fryguy
Copy link
Member

Fryguy commented Dec 19, 2023

@jrafanie Do you want this on quinteros?

@Fryguy
Copy link
Member

Fryguy commented Jan 5, 2024

Backported to quinteros in commit adfada4.

commit adfada4256670c870aa22f259efe4c6b71d5b7f6
Author: Jason Frey <[email protected]>
Date:   Tue Dec 19 14:33:45 2023 -0500

    Merge pull request #22801 from jrafanie/there_can_be_only_one_autoloader_zeitwerk
    
    There can be only one autoloader: zeitwerk
    
    (cherry picked from commit 7a3c4a686f445f852eb77a021e734507b889e11d)

Fryguy added a commit that referenced this pull request Jan 5, 2024
…der_zeitwerk

There can be only one autoloader: zeitwerk

(cherry picked from commit 7a3c4a6)
@jrafanie jrafanie deleted the there_can_be_only_one_autoloader_zeitwerk branch January 16, 2024 20:39
@jrafanie jrafanie mentioned this pull request Feb 22, 2024
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants