Adds rake task to cleanup compiled assets #1355
Conversation
|
/cc @josevalim |
|
Shouldn't the globbed filespec match that which spockets uses? i.e. Line 54 in Sprockets::StaticCompilation pattern = /^#{Regexp.escape(asset_pathname.basename_without_extensions.to_s)} rm_ing everything might not be what some people expect. |
|
Cool, but I'd suggest following rake conventions here. Call it |
|
done! /cc @josh |
| assets = Rails.application.config.assets | ||
| public_asset_path = Rails.public_path + assets.prefix | ||
| file_list = FileList.new("#{public_asset_path}/*.js", "#{public_asset_path}/*.css") | ||
| file_list.each{ |list| FileUtils.rm list } |
There was a problem hiding this comment.
Don't use FileUtils directly. Just do rm, its a helper built on to rake.
There was a problem hiding this comment.
Why only *.js and *.css? I think image assets should be cleaned up as well.
There was a problem hiding this comment.
So why *.js and *.css only? Also why all of them?
If the rake task tries to be smart by deleting only the "asset" files, then deleting all js/css doesn't make sense.
On the other hand, why not then just delete the whole assets directory thus making sure that all assets are cleared properly?
There was a problem hiding this comment.
Yeah, just change the glob to "#{public_asset_path}/**/*
There was a problem hiding this comment.
I updated the pull request as per this suggestion. Thanks, @josh
|
refectored!! /cc @josh |
Adds rake task to cleanup compiled assets
|
We need tests for this. Could someone please provide some tests? Otherwise I will have to revert it. |
|
@josevalim I will provide test case and other refactoring soon. /cc @josevalim |
|
@vatrai how were you thinking of testing it? I had considered moving the cleanup into a method on |
|
Tests can be added here: https://github.com/rails/rails/blob/master/railties/test/application/assets_test.rb There are examples of invoking the rake tasks in the same test case. |
|
Thanks, @josevalim! I'll get that done this morning and submit another pull request. |
|
I added some tests on my pull request. |
|
thanks!! @jamesarosen |
updated with requiring fileutils before attempting to use it.