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

Move config.assets out into sprockets-rails plugin #7981

Closed
wants to merge 1 commit into from

Conversation

josh
Copy link
Contributor

@josh josh commented Oct 18, 2012

Alright @rafaelfranca, got another thing.

It seems to work pretty good if the Sprockets Railtie just defines its own config.assets as an options set.

https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/railtie.rb#L48-55

app.config.assets.paths.unshift(*paths["vendor/assets"].existent_directories)
app.config.assets.paths.unshift(*paths["lib/assets"].existent_directories)
app.config.assets.paths.unshift(*paths["app/assets"].existent_directories)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I really hate doing this. How can we move this out too?

Copy link
Member

Choose a reason for hiding this comment

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

/cc @drogus @josevalim any ideas?

Copy link
Member

Choose a reason for hiding this comment

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

Can't we just add railtie to sprockets-rails and create an initializer there?

Copy link
Member

Choose a reason for hiding this comment

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

@drogus in this case not. This is a initializer that have to run in every engine. I think we can move this to sprockets-rails, reopen Rails::Engine there and add this initializer, since the sprockets/railtie is loaded before the engines.

@josevalim
Copy link
Contributor

I don't think we should completely remove the config.assets from Rails. For example, config.assets.paths can be consumed by other applications (I believe rake-pipeline does it). So I would rather leave the basic options around and sprockets can extend it further with its own.

@josh
Copy link
Contributor Author

josh commented Oct 18, 2012

@josevalim the annoying issue is that assets is attr_accessor instead of a property on the ordered options object. Inorder to define my own options, I need to undef that accessor.

I think rake-pipeline should just bring its own options that it cares about.

@josh
Copy link
Contributor Author

josh commented Oct 18, 2012

Alright, don't really care now, I'm going to have to wipe the current config.assets anyway for old rails. So whatever config.assets are initialized by rails will just be ignored by the plugin.

@josh josh closed this Oct 18, 2012
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

5 participants