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

Move node-sass to npm script and drop Ruby Sass. #21489

Merged
merged 2 commits into from
Jan 4, 2017
Merged

Move node-sass to npm script and drop Ruby Sass. #21489

merged 2 commits into from
Jan 4, 2017

Conversation

bardiharborow
Copy link
Member

This PR moves node-sass to a NPM script.

Per #20332, I've also dropped Ruby Sass support:

Compile Sass files with node-sass (no more Ruby options)

@hnrch02
Copy link
Collaborator

hnrch02 commented Jan 1, 2017

Do we know if libsass finally achieved feature parity with sass? A comparison in output for the unminified file would be handy, I think. (Unless this has already been examined and I'm OOTL.)

@bardiharborow
Copy link
Member Author

Bootstrap already uses libsass by default, so this doesn't change that. The only differences in the output between libsass and Ruby Sass are to do with how whitespace is treated (I had to use --ignore-blank-lines to avoid diffing hundreds of added lines.) As for whether this will disrupt anyone, I posted a heads up on Slack and haven't heard any complaints.

Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do it! Looks like tests are failing for some linting issues, otherwise looks good to me!

@bardiharborow bardiharborow requested a review from hnrch02 January 3, 2017 00:53
@bardiharborow
Copy link
Member Author

bardiharborow commented Jan 3, 2017

@hnrch02 did you have any further concerns or shall we LGTM? I've fixed the linting issue.

Copy link
Collaborator

@hnrch02 hnrch02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like I was just out of the loop on ruby Sass 😄 LGTM!

@bardiharborow bardiharborow merged commit dd19670 into twbs:v4-dev Jan 4, 2017
@bardiharborow bardiharborow deleted the npm-sass branch January 4, 2017 01:28
@mdo mdo mentioned this pull request Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants