-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat: add default config for stylelint #224
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple notes, mainly style choices and testing semantics. Also looks like you have a couple tests failing, not immediately sure why but shouldn't be too hard to reason about. Ping me when ready for re-review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For [lack of forethought and for simplicity] reasons, all test input files must have different prefixes, even if they have different file extensions. Please rename one of these. I usually prefix as basic_py, or similar. You'll notice that none of these snapshots contain "basic.in.sass" for this reason.
# When enabling stylelint, you should also add the stylelint plugin configs, | ||
# like so: | ||
# | ||
# lint: | ||
# enabled: | ||
# - stylelint@15.3.0: | ||
# packages: | ||
# - stylelint-config-standard@31.0.0 | ||
# - stylelint-config-standard-scss@7.0.1 | ||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that this is strictly an improvement, but I'm not sure if this is discoverable enough. Thoughts on moving this to a linters/stylelint
-level readme or perhaps somewhere else?
# packages: | ||
# - stylelint-config-standard@31.0.0 | ||
# - stylelint-config-standard-scss@7.0.1 | ||
# | ||
definitions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because you've added a direct config, you must set is_recommended: false
(this is causing the repo_tests failure)
Also make stylelint work with scss