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

Don't declare helper_method if the method is not supported #748

Closed
wants to merge 2 commits into from

Conversation

Empact
Copy link
Contributor

@Empact Empact commented Jul 19, 2017

Eg. ActionController::API does not support helper_method

Eg. ActionController::API does not support helper_method
end

def post(path, params=nil, headers=nil)
super(path, params: params, headers: headers)
def post(path, params=nil)

Choose a reason for hiding this comment

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

Surrounding space missing in default value assignment.

end

def put(path, params=nil, headers=nil)
super(path, params: params, headers: headers)
def put(path, params=nil)

Choose a reason for hiding this comment

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

Surrounding space missing in default value assignment.

@@ -3,16 +3,16 @@
# supported in Rails 5+. Since we support back to 3.1, we need some sort of shim
# to avoid super noisy deprecations when running tests.
module HTTPMethodShim
def get(path, params=nil, headers=nil)
super(path, params: params, headers: headers)
def get(path, params=nil)

Choose a reason for hiding this comment

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

Surrounding space missing in default value assignment.

it "exposes no action methods" do
expect(controller.action_methods).to be_empty
if defined?(ActionController::API)
context 'included within ActionController::API' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -1,11 +1,25 @@
require "spec_helper"

describe Clearance::Controller, type: :controller do
controller(ActionController::Base) do
include Clearance::Controller
context 'included within ActionController::Base' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

end

def post(path, params=nil, headers=nil)
super(path, params: params, headers: headers)
def post(path, params=nil)

Choose a reason for hiding this comment

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

Surrounding space missing in default value assignment.

end

def put(path, params=nil, headers=nil)
super(path, params: params, headers: headers)
def put(path, params=nil)

Choose a reason for hiding this comment

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

Surrounding space missing in default value assignment.

@@ -3,16 +3,16 @@
# supported in Rails 5+. Since we support back to 3.1, we need some sort of shim
# to avoid super noisy deprecations when running tests.
module HTTPMethodShim
def get(path, params=nil, headers=nil)
super(path, params: params, headers: headers)
def get(path, params=nil)

Choose a reason for hiding this comment

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

Surrounding space missing in default value assignment.

it "exposes no action methods" do
expect(controller.action_methods).to be_empty
if defined?(ActionController::API)
context 'included within ActionController::API' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -1,11 +1,25 @@
require "spec_helper"

describe Clearance::Controller, type: :controller do
controller(ActionController::Base) do
include Clearance::Controller
context 'included within ActionController::Base' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

include Clearance::Controller
end

it 'exposes no action methods' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

include Clearance::Controller
end

it 'exposes no action methods' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

include Clearance::Controller
end

it 'exposes no action methods' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

include Clearance::Controller
end

it 'exposes no action methods' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

include Clearance::Controller
end

it 'exposes no action methods' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

include Clearance::Controller
end

it 'exposes no action methods' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

…troller

This required:
* Dropping header support from the http_method_shim. Rails 5 does not have an equivalent argument
* Explicitly adding rails-controller-testing as an external dependency
* Bumping to rspec-rails ~>3.5, as earlier versions do not support Rails 5
@derekprior
Copy link
Contributor

Thank you. Simplified and merged as 56443e6. I will release soon (hopefully by Friday at the latest).

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