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

Allow no compression for all environments #338

Merged
merged 1 commit into from
Sep 3, 2015

Conversation

jsilvestri
Copy link
Contributor

Change 339529f made it so that one can no longer turn off compression in any environment except development. This is both a bit confusing (e.g. you set it to nil and expect it to work, but it doesn't!) and difficult if you really want to turn compression off. This change allows the statement config.assets.css_compressor = nil to function in all environments.

@byroot
Copy link
Member

byroot commented Jul 17, 2015

👍

@rafaelfranca
Copy link
Member

Why just for development. If we are going to make it configurable better to do for all environment. Mind to change that?

@jsilvestri
Copy link
Contributor Author

@rafaelfranca I'm not quite sure I follow you. My change is to make it so that one can set app.config.assets.css_compressor = nil in all environments. The former code only allows you to do so on development. The code might look a little cleaner this way -- is this what you were thinking?

if Rails.env.development?
  # For development, default to an expanded output instead of the sass default of :nested
  app.config.sass.style ||= :expanded
else
  # Default to compress css in non-development environments
  # while still allowing it to be overridden to nil
  app.config.assets.css_compressor = :sass unless app.config.assets.has_key?(:css_compressor)
end

@jsilvestri
Copy link
Contributor Author

@rafaelfranca / @byroot any more thoughts?

@byroot
Copy link
Member

byroot commented Sep 3, 2015

Yeah sorry @jsilvestri. Totally looks good to me!

@jsilvestri
Copy link
Contributor Author

@byroot I don't have merge privileges -- are you able to merge?

@byroot
Copy link
Member

byroot commented Sep 3, 2015

@rafaelfranca will merge if it looks good to him too. Don't worry :)

rafaelfranca added a commit that referenced this pull request Sep 3, 2015
Allow no compression for all environments
@rafaelfranca rafaelfranca merged commit 3616c29 into rails:master Sep 3, 2015
@rafaelfranca
Copy link
Member

Thank you!

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