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.

Sign up for GitHub

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

Jump to bottom

assets version doesn't depend on asset host if it's a Proc #140

Merged
merged 1 commit into from Apr 22, 2014
Merged

assets version doesn't depend on asset host if it's a Proc #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/sprockets/railtie.rb
View file
Open in desktop
Expand Up @@ -74,8 +74,8 @@ def configure(&block)
app.assets.version,
config.assets.version,
config.action_controller.relative_url_root,
config.action_controller.asset_host,
Sprockets::Rails::VERSION,
(config.action_controller.asset_host unless config.action_controller.asset_host.respond_to?(:call)),
Copy link
Member

Choose a reason for hiding this comment

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

As we've already got a compact on this array, I think we can just use an inline conditional...

config.action_controller.asset_host unless config.action_controller.asset_host.respond_to?(:call),

wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can, but I found inline condition inside an array too complex, it doesn't even fit one line )

Copy link
Member

Choose a reason for hiding this comment

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

I prefer @matthewd version than doing the operation two times.

Sprockets::Rails::VERSION
].compact.join('-')

# Copy config.assets.paths to Sprockets
Expand Down
16 changes: 15 additions & 1 deletion test/test_railtie.rb
View file
Open in desktop
Expand Up @@ -112,7 +112,7 @@ def test_version
assert_equal "test-v2-#{Sprockets::Rails::VERSION}", env.version
end

def test_version_fragments
def test_version_fragments_with_string_asset_host
app.configure do
config.assets.version = 'v2'
config.action_controller.asset_host = 'http://some-cdn.com'
Expand All @@ -124,6 +124,20 @@ def test_version_fragments
assert_equal "test-v2-some-path-http://some-cdn.com-#{Sprockets::Rails::VERSION}", env.version
end

def test_version_fragments_with_proc_asset_host
app.configure do
config.assets.version = 'v2'
config.action_controller.asset_host = ->(path, request) {
'http://some-cdn.com'
}
config.action_controller.relative_url_root = 'some-path'
end
app.initialize!

assert env = app.assets
assert_equal "test-v2-some-path-#{Sprockets::Rails::VERSION}", env.version
end

def test_configure
app.configure do
config.assets.configure do |env|
Expand Down