Allow no compression for all environments by jsilvestri · Pull Request #338 · rails/sass-rails (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation8 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

jsilvestri

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.

@jsilvestri

@byroot

@rafaelfranca

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

@jsilvestri

@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

@byroot

@jsilvestri

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

@byroot

@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

@rafaelfranca

Allow no compression for all environments

@rafaelfranca