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

Add sass linting #228

Closed
rpowis opened this issue Oct 8, 2015 · 7 comments
Closed

Add sass linting #228

rpowis opened this issue Oct 8, 2015 · 7 comments

Comments

@rpowis
Copy link
Contributor

rpowis commented Oct 8, 2015

...to keep things tidy.

https://github.com/sasstools/sass-lint

@rpowis
Copy link
Contributor Author

rpowis commented Jan 14, 2016

@rpowis rpowis closed this as completed Jan 14, 2016
@rpowis rpowis reopened this Jan 14, 2016
@rpowis
Copy link
Contributor Author

rpowis commented Jan 14, 2016

oops

@feedmypixel
Copy link
Contributor

Have just done a setup with sass and https://github.com/sasstools/sass-lint using the rule

property-units:
    - 2
    - global:
        - px

And then using post-css with the plugin https://github.com/cuth/postcss-pxtorem to convert px to rem.
In this way we could lint values being used in *.scss and convert after rather than using mixins.

@rpowis
Copy link
Contributor Author

rpowis commented Apr 23, 2016

@feedmypixel Having considered this some more, I think we'd be better off moving margins and paddings into variables with em values (can't use rems due to support) and enforcing the use of those through linting.

It feels like a very heavy handed approach though, but I worry that by allowing the use of arbitrary magic numbers all over the place we'll struggle to maintain consistency in components and layouts.

@rpowis
Copy link
Contributor Author

rpowis commented Apr 23, 2016

Also, we should probably consider using stylelint over sass-lint for it's modularity (e.g. stylelint-selector-bem-pattern). Much the same as eslint for js

@rpowis rpowis changed the title Add sass-lint Add sass linting Apr 23, 2016
@feedmypixel
Copy link
Contributor

👍 on stylelint

we could

  • add the stylelint rule: unit-whitelist without “px”
  • use variables set in px (for ease of communications with designers and translation of designs)
  • disable/ignore vars file that contains $baseline and $gutter px vars (this being the only place to add them)
  • use postcss-px-to-em https://github.com/macropodhq/postcss-px-to-em for support

note we could also just not replace px values with rem when using pxtorem it would be interesting to see how much weight this adds to the stylesheets

Potentially we would also write a plugin so people can’t /* variables
This would then stop people using magic numbers for padding and margin. $baseline\1.2 or $baseline*3.76

More of a guidance than a heavy handed approach

@rpowis rpowis mentioned this issue May 19, 2016
@rpowis rpowis closed this as completed Jun 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants