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

Support Rails 5 #1483

Merged
merged 4 commits into from
Aug 1, 2016
Merged

Support Rails 5 #1483

merged 4 commits into from
Aug 1, 2016

Conversation

cbeer
Copy link
Member

@cbeer cbeer commented Jul 8, 2016

Fixes #1468

@coveralls
Copy link

coveralls commented Jul 8, 2016

Coverage Status

Coverage decreased (-0.2%) to 95.877% when pulling b44b13b on rails5 into 4014aef on master.

@@ -14,5 +14,9 @@ class MastheadUploader < CarrierWave::Uploader::Base
def store_dir
"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
end

def default_url
ActionController::Base.helpers.asset_path("fallback/" + [version_name, "default.png"].compact.join('_'))

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@search.attributes = search_params
@search.query_params = params.except(:exhibit_id, :search, *blacklisted_search_session_params).reject { |_k, v| v.blank? }
@search.attributes = search_params.
@search.query_params = params.to_unsafe_h.except(:exhibit_id, :search, *blacklisted_search_session_params).reject { |_k, v| v.blank? }

Choose a reason for hiding this comment

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

unexpected token tIVAR
(Using Ruby 2.3 parser; configure using TargetRubyVersion parameter, under AllCops)

Copy link
Member

Choose a reason for hiding this comment

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

Looks like a trailing dot on this line.

initialize_routing_helpers(helper)
end

def initialize_routing_helpers(helpers)

Choose a reason for hiding this comment

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

Unused method argument - helpers. If it's necessary, use _ or _helpers as an argument name to indicate that it won't be used. You can also write as initialize_routing_helpers(*) if you want the method to accept any arguments but don't care about them.

@cbeer cbeer force-pushed the rails5 branch 3 times, most recently from 0cde78c to 8b4e470 Compare July 29, 2016 22:17
@@ -13,7 +13,7 @@ class SearchesController < Spotlight::ApplicationController

def create
@search.attributes = search_params
@search.query_params = params.except(:exhibit_id, :search, *blacklisted_search_session_params).reject { |_k, v| v.blank? }
@search.query_params = params.to_unsafe_h.with_indifferent_access.except(:exhibit_id, :search, *blacklisted_search_session_params).reject { |_k, v| v.blank? }

Choose a reason for hiding this comment

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

Line is too long. [164/160]

@cbeer cbeer force-pushed the rails5 branch 2 times, most recently from 590794c to 6236925 Compare July 30, 2016 00:20
@coveralls
Copy link

coveralls commented Jul 30, 2016

Coverage Status

Coverage increased (+0.003%) to 96.119% when pulling 6236925 on rails5 into 30d0cb6 on master.

@cbeer cbeer changed the title [WIP] Support Rails 5 Support Rails 5 Jul 30, 2016
@jcoyne
Copy link
Member

jcoyne commented Jul 30, 2016

Should we as to the README that you need particular branches of the dependencies to use it with Rails 5

@cbeer
Copy link
Member Author

cbeer commented Jul 30, 2016

Possibly a good thing to discuss next week. Hopefully it's down to just two sources now, and I'd be optimistic that friendly_id will get a new release fairly soon, and I wonder if we should drop social-share-button entirely.

@coveralls
Copy link

coveralls commented Jul 30, 2016

Coverage Status

Coverage increased (+0.003%) to 96.119% when pulling 018a0fb on rails5 into 30d0cb6 on master.

@coveralls
Copy link

coveralls commented Aug 1, 2016

Coverage Status

Coverage increased (+0.003%) to 96.119% when pulling b40afd1 on rails5 into 01c210d on master.

@jcoyne jcoyne merged commit caee862 into master Aug 1, 2016
@jcoyne jcoyne deleted the rails5 branch August 1, 2016 17:05
@jcoyne jcoyne removed the in progress label Aug 1, 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.

4 participants