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

2.0.0: Remove dependency on Ruby Sass #85

Merged
merged 6 commits into from Sep 26, 2018

Conversation

csuhta
Copy link
Contributor

@csuhta csuhta commented Aug 30, 2018

This is a WIP pull request, but I would like to get your thoughts on these things, some of them still in progress:

  • Removes dependency on Ruby Sass (sass)
  • Require Ruby 2.3.3 or higher
  • Drop test support for Compass (Compass is EOL)
  • Bump the version to 2.0.0 because of the above changes
  • Use our own SassC::SyntaxError to replace the Sass error class tree
  • Remove CacheStores (this doesn't seem to be needed anymore?)
  • Steal a lot of SassC::Script::Value::* class code from Ruby Sass to free the dependency. This code also includes a lot of functionality to implement Ruby Sass properly, not just the FFI interface, so I need to finish auditing all of it for what we don't actually need to bring to this gem
  • Should test on more versions of Ruby

cc #75

@bolandrm
Copy link
Member

bolandrm commented Sep 6, 2018

@csuhta I know there may be some extraneous code here in the script/values files, but what do you think about merging this (since the dependency has been eliminated)? We could potentially go back and remove unused code at a later time.

@csuhta
Copy link
Contributor Author

csuhta commented Sep 7, 2018

@bolandrm Tests pass, so if you are happy with it currently, we can merge. (You're also totally good with the 2.0.0 version bump and the new requirement to use Ruby 2.3.3 or higher too?)

@bolandrm
Copy link
Member

bolandrm commented Sep 7, 2018

@csuhta why exactly do we need to specify the ruby version?

@csuhta
Copy link
Contributor Author

csuhta commented Sep 10, 2018

@bolandrm Without required_ruby_version in the Gemspec, the gem asserts by default that any version of Ruby can try to use it. It will likely not work on any Ruby less than 2.0 or 2.1. The oldest version tested in the Travis config is 2.3.3, so that's what I set as the minimum.

@csuhta
Copy link
Contributor Author

csuhta commented Sep 26, 2018

@bolandrm I've pushed a few more changes and I think this is in much better shape:

  • Greatly trimmed down surface area of the SassC::Script::Value::Color class
  • Trimmed down the surface area of the SassC::Util module
  • Now testing on the latest Ruby 2.3.x stable
  • The whole project has been frozen_string_literal'd
  • Slightly less memory is used now

@bolandrm bolandrm merged commit 61a02d1 into sass:master Sep 26, 2018
@bolandrm
Copy link
Member

🎉 thanks @csuhta !!

This was referenced Sep 26, 2018
DirtyF added a commit to jekyll/jekyll-sass-converter that referenced this pull request Oct 1, 2018
Remove dependency on Ruby Sass
sass/sassc-ruby#85
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.

None yet

2 participants