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

Require Sass::Deprecation module #68

Merged
merged 1 commit into from
Jul 13, 2017

Conversation

beagleknight
Copy link
Contributor

Hi there!

I was developing a rails application using the sassc-rails gem and I got an error like this:

NameError: uninitialized constant Sass::Deprecation

Then I realized that this gem was including the latest version of sass and it is requiring the modules individually in the lib/sassc/script.rb file.

I am not sure if this fix belongs here or the require is missing in some sass file.

Thoughts?

@deivid-rodriguez
Copy link
Contributor

Yeah, the alternative would be sass requiring sass/deprecation every time it's used. That seems more explicit to me.

@fschwahn fschwahn mentioned this pull request Jul 10, 2017
@meowsus
Copy link

meowsus commented Jul 10, 2017

@deivid-rodriguez I believe I did that same alternative in #70

This resolves #69

@meowsus
Copy link

meowsus commented Jul 10, 2017

@beagleknight for reference sass/sass#2338

@deivid-rodriguez
Copy link
Contributor

deivid-rodriguez commented Jul 10, 2017

sass has rejected the alternative patch, so the fix needs to happen here.

Given that sass does not support current usage, maybe it's better to require the whole of sass to avoid future breakage.

@meowsus
Copy link

meowsus commented Jul 10, 2017

@deivid-rodriguez that would be more futureproof, for sure, but I'm curious what the negative effects of this might be.

@bolandrm is there a reason why we're picking and choosing sass functionality and not requiring the whole library?

@meowsus
Copy link

meowsus commented Jul 11, 2017

@bolandrm I don't mean to be a bother, but I was curious if you had an idea when of when we might expect a patch. This issue seems to be affecting a good number of folks and the entirety of our platform's integrators. Instead of having us patch manually, we were hoping for a fix from upstream.

Thanks, and sorry for the prodding.

@robinboening
Copy link

I also ran into this issue yesterday. Would be great to see this / a patch merged and a new release. Thanks folks!

andrew added a commit to librariesio/libraries.io that referenced this pull request Jul 13, 2017
@bolandrm bolandrm merged commit f903118 into sass:master Jul 13, 2017
@bolandrm
Copy link
Member

Thanks all - please try out 1.11.4 and let me know if there are any issues.

@robinboening
Copy link

Thanks! I will try and report back.

@beagleknight
Copy link
Contributor Author

Thank you! 😄

@meowsus
Copy link

meowsus commented Jul 13, 2017

Yes! Thank you! I can confirm that this took care of my error.

@beagleknight beagleknight deleted the fix/sass-deprecation branch July 13, 2017 13:49
@robinboening
Copy link

I can also confirm it works with v1.11.4 now! Thanks a lot!

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.

5 participants