Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix(mixins): removed multilined comment #9002

Closed
wants to merge 1 commit into from

Conversation

EladBezalel
Copy link
Member

@EladBezalel EladBezalel commented Jul 11, 2016

  • some node versions with node-sass on windows can't compile it into 1 line

@EladBezalel EladBezalel added the needs: review This PR is waiting on review from the team label Jul 11, 2016
@EladBezalel EladBezalel force-pushed the fix/multiline-comments-scss branch from 9860764 to 9488fa4 Compare July 11, 2016 18:04
@devversion
Copy link
Member

LGTM! - Also once thing I've noticed. It's actually not affecting the bower-material modules, due to the fact that the themeBuildStream function only runs for the core modules and general JS output.

@@ -205,6 +205,6 @@ function themeBuildStream() {
return gulp.src( config.themeBaseFiles.concat(path.join(config.paths, '*-theme.scss')) )
.pipe(concat('default-theme.scss'))
.pipe(utils.hoistScssVariables())
.pipe(sass())
Copy link
Contributor

@ThomasBurleson ThomasBurleson Jul 11, 2016

Choose a reason for hiding this comment

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

Node-sass does not like this:

stream.js:74
      throw er; // Unhandled stream error in pipe.
      ^
Error: src/components/autocomplete/demoBasicUsage/index.html
Error: Invalid CSS after "...emoBasicUsage {": expected "}", was "<div ng-controller="
        on line 1 of src/components/autocomplete/demoBasicUsage/index.html
>> .autocompletedemoBasicUsage {
   -----------------------------^

  at options.error (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/node-sass/lib/index.js:272:32)

Using node 6.3.

- some node versions with `node-sass` on windows can't compile it into 1 line
@EladBezalel EladBezalel force-pushed the fix/multiline-comments-scss branch from 9488fa4 to c4eb955 Compare July 18, 2016 17:58
@devversion
Copy link
Member

@ThomasBurleson Still LGTM - Tracking the original issue with #9066

@devversion devversion removed their assignment Jul 18, 2016
@EladBezalel EladBezalel changed the title fix(gulp-util): handle multiline comments in scss fix(mixins): removed multilined comment Jul 18, 2016
@ThomasBurleson ThomasBurleson added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Jul 19, 2016
@EladBezalel EladBezalel deleted the fix/multiline-comments-scss branch July 20, 2016 19:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants