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

[infrastructure] Add (S)CSS linting infrastructure #4564

Merged
merged 1 commit into from
Jul 20, 2016

Conversation

traviskaufman
Copy link
Contributor

  • Add Stylelint config
  • Add BEM linting
  • Add SCSS linting
  • Fix all general lint errors
  • Remove theme scss files as we won't be using them moving forward

Part of #4464
Relates to #4539

NOTE: Assigning everyone since this should get feedback from all core team members

# reason to make root a compound selector (?)
selector-root-no-composition: true
# http://www.paulirish.com/2010/the-protocol-relative-url/
function-url-no-scheme-relative: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should actually enforce using HTTPS wherever possible, not relative URLs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

AH, OK. I was searching around and saw the initial issue. They were saying relative URLs there were good and should be enforced, so I thought this was doing that instead.

@Garbee
Copy link
Collaborator

Garbee commented Jul 18, 2016

LGTM

@rfru
Copy link

rfru commented Jul 18, 2016

LGTM
Very nice!

What are the // postcss-bem-linter: comments used for? (besides ignoring)

@traviskaufman
Copy link
Contributor Author

@rfru it tells postcss-bem-linter that the code contained in between them is a BEM component - https://github.com/postcss/postcss-bem-linter#defining-a-component

I put the end comments in to avoid potential issues with sass concatenating files together and having the linter throw false positives.

# http://stackoverflow.com/q/3851091
selector-attribute-quotes: always
# Single-quotes are a convention throughout our codebase
string-quotes: single
Copy link
Contributor

Choose a reason for hiding this comment

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

We've been doing double quotes in CSS and single quotes in JS, but I suppose single quotes everywhere works too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay no problem. I can change this. I don't mind dbl-quotes anyway. They look like attributes and that's what's used in the WebIDL for attrs (AFAIK).

@sgomes
Copy link
Contributor

sgomes commented Jul 20, 2016

LGTM, can't believe the range of options available!

* Add Stylelint config
* Add BEM linting
* Add SCSS linting
* Fix all general lint errors
* Remove theme scss files as we won't be using them moving forward

Part of #4464
Relates to #4539
@traviskaufman traviskaufman merged commit 0eccbee into master Jul 20, 2016
@traviskaufman traviskaufman deleted the chore/build-lint-infra-css branch July 20, 2016 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants