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

Fix #0d31e72 regression #611

Merged
merged 1 commit into from
Aug 26, 2014
Merged

Conversation

gdlx
Copy link

@gdlx gdlx commented Aug 26, 2014

Fix for 0d31e72 regression (see comments on initial commit).

steveklabnik added a commit that referenced this pull request Aug 26, 2014
@steveklabnik steveklabnik merged commit d5a993e into rails-api:0-9-stable Aug 26, 2014
@steveklabnik
Copy link
Contributor

Thanks! I'll cut a 0.9.1 shortly.

@gdlx gdlx deleted the Fix-0d31e72 branch August 26, 2014 15:42
@byroot
Copy link

byroot commented Aug 27, 2014

Sorry for the bug guys, but on_load(:action_controller) will not work. on_load(:after_initialized) is required otherwise you cannot disable the controller extensions from you application.

I'll try to find a fix for that.

Again sorry for the bug.

@gdlx
Copy link
Author

gdlx commented Aug 27, 2014

@byroot That means your way of disabling controller extensions will not work: :after_initialized is called after controllers are loaded. A controller calling an extension method will crash if the extension isn't loaded yet.

A clean disabling of controller extensions would be to create 3 gems : one for serializers, one for controller extensions and another to group them in a single gem if both are needed...lot of work for what seems to be an uncommon need...

Have you ever seen another gem disabling some features the way you did ? If you do, maybe we could find the little trick we miss...

Another question I have is: why do you need to avoid loading controller extensions ? Just because you don't need them or because they're causing trouble in your project ?

The only overloaded method in controller extensions is _render_option_json, which just calls super when no serializer is returned by build_json_serializer. Maybe you could just overload build_json_serializer in an initializer to never return a serializer, that way keeping the default Rails behavior...

@byroot
Copy link

byroot commented Aug 27, 2014

why do you need to avoid loading controller extensions ?

Because they don't play well with our legacy custom responders, and that on a 10 years old application it's not easy to migrate to AMS overnight so we need a transitory solution.

Maybe you could just overload build_json_serializer

Good point I will investigate that, thanks.

@gdlx
Copy link
Author

gdlx commented Aug 27, 2014

Ok, please keep us updated with your test. If it succeeds, it would be nice to revert your PR, and update doc with an how-to for disabling controller extensions !

@steveklabnik
Copy link
Contributor

Agreed.

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