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

Fixed lazy load action controller #206

Merged

Conversation

ericproulx
Copy link
Contributor

I found that the ActiveSupport.on_load(:action_controller) wasn't working as intented because action_controller was require on top.

I figured that requiring test_case only when needed would fix that small issue

@coveralls
Copy link

coveralls commented Mar 2, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 47c6c44 on ericproulx:fix_lazy_load_action_controller into 93b3713 on enriclluelles:master.

@tagliala tagliala self-requested a review March 2, 2020 12:18
Copy link
Collaborator

@tagliala tagliala left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.

I'm going to merge and release a new version of the gem, just please use the imperative verb in the commit title (Move rather than Moved)

Require TestCase only if test environment
Remove action_controller require in extensions/action_controller
@ericproulx ericproulx force-pushed the fix_lazy_load_action_controller branch from 9eb1b6d to 47c6c44 Compare March 2, 2020 12:31
@tagliala tagliala self-requested a review March 2, 2020 13:19
@tagliala tagliala merged commit 7785c8d into enriclluelles:master Mar 2, 2020
@tagliala tagliala added this to the 7.1.2 milestone Mar 2, 2020
@tagliala
Copy link
Collaborator

tagliala commented Mar 2, 2020

@ericproulx thanks, merged and released in 7.1.2

Please give a try and let me know if there are any issues

@ericproulx ericproulx deleted the fix_lazy_load_action_controller branch May 14, 2020 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants