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

Preload Gaffee::ErrorsController in spec_helper fixes #31 #33

Merged
merged 1 commit into from
Apr 18, 2016

Conversation

uzzer
Copy link
Contributor

@uzzer uzzer commented Apr 14, 2016

No description provided.

@remi
Copy link
Member

remi commented Apr 14, 2016

I don’t think this is the best way to solve this. I’d rather add this after the require 'gaffe' inspec_helper.rb`:

# We need a fake "ApplicationController" because Gaffe's default controller inherits from it
class ApplicationController < ActionController::Base
end

require 'gaffe/errors_controller'

than add something in lib/gaffe. If you want to fix this in your PR, I’ll merge it 😄

@uzzer
Copy link
Contributor Author

uzzer commented Apr 14, 2016

It will brake beautiful require block area. Perhaps we can deal somehow else with fake application_controller?

@remi
Copy link
Member

remi commented Apr 14, 2016

We could store the fake controller in spec/application_controller_helper.rb and require it in spec_helper.rb:

require 'rspec'
require 'action_controller/railtie'
require 'gaffe'
require_relative './application_controller_helper'
require 'gaffe/errors_controller'

@uzzer
Copy link
Contributor Author

uzzer commented Apr 14, 2016

Agree

@remi
Copy link
Member

remi commented Apr 14, 2016

Squash that into a single commit and we’re good to go! 😄

…commit)

Squashed commits:
[8a97b6c] Preload Gaffee::ErrorsController in spec_helper fixes mirego#31
@uzzer
Copy link
Contributor Author

uzzer commented Apr 14, 2016

Squashed

Perparing commit

@uzzer
Copy link
Contributor Author

uzzer commented Apr 18, 2016

@remiprev any updates?

@remi
Copy link
Member

remi commented Apr 18, 2016

All good! Thanks!

@remi remi merged commit e569e94 into mirego:master Apr 18, 2016
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.

None yet

2 participants