Skip to content

Stop overriding the app css_compressor setting #32

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

Merged
merged 1 commit into from
Jul 17, 2015

Conversation

byroot
Copy link
Contributor

@byroot byroot commented Jul 17, 2015

I fail to understand why this gem is doing so, but it's preventing users to configure the CSS compressing as they will.

@bolandrm could we remove it, or am I missing something?

cc @edward

@bolandrm
Copy link
Member

Hey, looks like a bug. I believe the only line that needs to be removed is Line 56.

@byroot
Copy link
Contributor Author

byroot commented Jul 17, 2015

Hey, looks like a bug. I believe the only line that needs to be removed is Line 56.

No, because nil mean you don't want any compressor. For example we don't want compression in our test environment.

@bolandrm
Copy link
Member

After removing line 56, the API becomes the same as sass-rails. sass-rails appears to force compression in the test environment.

@byroot
Copy link
Contributor Author

byroot commented Jul 17, 2015

God you're right. Let's do like sass-rails then.

But ping @rafaelfranca: Do you think this is a same default: https://github.com/rails/sass-rails/blob/master/lib/sass/rails/railtie.rb#L69-L76 ?

@bolandrm
Copy link
Member

@byroot I can incorporate this PR:

https://github.com/rails/sass-rails/pull/338/files

so that we allow no compression in the test environment.

@byroot byroot force-pushed the stop-messing-with-css-compressor branch from d2fa327 to 67ebcf9 Compare July 17, 2015 18:01
@byroot
Copy link
Contributor Author

byroot commented Jul 17, 2015

Sounds like a good idea. Maybe we should wait to see what sass-rails does of this PR?

In the meantime I updated this PR to copy sass-rails behaviour, and I fixed the test suite.

bolandrm added a commit that referenced this pull request Jul 17, 2015
Stop overriding the app css_compressor setting
@bolandrm bolandrm merged commit 441aaed into sass:master Jul 17, 2015
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.

2 participants