Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

chore(sass-lint): add sass linting #3732

Merged
merged 1 commit into from
May 16, 2016
Merged

Conversation

TDA
Copy link
Contributor

@TDA TDA commented May 12, 2016

Add sass-lint and grunt-sass-lint to our workflow.

Fixes #3372
@pdehaan

@TDA TDA force-pushed the issue-3372-sass-linting branch 2 times, most recently from a73a51d to 3c22f8e Compare May 12, 2016 22:16
@pdehaan
Copy link
Contributor

pdehaan commented May 12, 2016

  1. Sass refactor looks solid.
  2. Don't see a npm-shrinkwrap.json edit, but I can't remember if this repo shrinkwraps devDependencies.
  3. Don't see anybody consuming the new sasslint Grunt task. Not sure if we should add it to grunttasks/lint.js so it gets triggered on every PR — to keep everybody honest.

@pdehaan
Copy link
Contributor

pdehaan commented May 12, 2016

Plus, make sure we remove unused /app/styles/.scss-lint.yml file.

remove unused .scss-lint.yml file
add sasslint to grunt lint task
@TDA TDA force-pushed the issue-3372-sass-linting branch from 3c22f8e to c885ef6 Compare May 12, 2016 22:48
@TDA
Copy link
Contributor Author

TDA commented May 12, 2016

@pdehaan Removed the unused file, and added the sasslint to lint tasks.

@pdehaan
Copy link
Contributor

pdehaan commented May 12, 2016

LGTM.
I'll let @vladikoff @shane-tomlinson @philbooth @vbudhram do the final r? and pull the 🔫 .

@@ -1,4 +1,6 @@
.avatar-spinner {
@extend %fill-avatar;
Copy link
Contributor

Choose a reason for hiding this comment

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

need to check this visually

@vladikoff vladikoff assigned vladikoff and unassigned TDA May 16, 2016
@vladikoff vladikoff merged commit c4d2def into master May 16, 2016
@vladikoff vladikoff deleted the issue-3372-sass-linting branch May 16, 2016 22:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants