-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Make Controller Helpers autoloadable #6062
Make Controller Helpers autoloadable #6062
Conversation
ea50bb9
to
1eced48
Compare
I think the reason to have them in lib is to being easily loadable by host applications and extensions. Can we do the same now? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6062 +/- ##
==========================================
- Coverage 89.47% 89.28% -0.19%
==========================================
Files 805 813 +8
Lines 17875 17906 +31
==========================================
- Hits 15993 15987 -6
- Misses 1882 1919 +37 ☔ View full report in Codecov by Sentry. |
Yes, they have the same loading characteristics as e.g. |
Right, we just don't need any require before using it now. What about keeping the files and print a warning telling people that they can remove the require now? I'm afraid this might break all stores requiring the lib files. |
39e835b
to
38e9282
Compare
Good catch. I've re-added the files in |
38e9282
to
cf07217
Compare
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.
@mamhoff approved and auto-merge enabled. Once the conflict is solved, it will be merged. |
Head branch was pushed to by a user without write access
024a4e4
to
47888bd
Compare
47888bd
to
e65f550
Compare
There's no real good reason to keep these files in lib and require them on app startup. Let's put them in app/ and let Zeitwerk handle their loading.
These are easy enough.
e65f550
to
0ef2e29
Compare
Thanks for also tackling the Rubocop todos |
There's no real good reason to keep these files in
lib
and require them on app startup. Let's put them inapp/
and let Zeitwerk handle their loading.