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

Allow !default annotation #132

Merged
merged 4 commits into from
Oct 24, 2022

Conversation

Hyzual
Copy link
Contributor

@Hyzual Hyzual commented Sep 27, 2022

This fixes #131

@kristerkari
Copy link
Collaborator

Hmm.. looks like the tests are broken, maybe on master too. Need to have a look at that.

@jnoordsij
Copy link
Contributor

Seeing messages like "Unknown rule annotation-no-unknown", which suggest #117 might be required first.

Side note: you might want to consider replacing tests on Node v12 with tests on Node v18.

@kristerkari
Copy link
Collaborator

Merged #117, so this needs to be rebased against the current master. Also, an example could be added to valid.scss file.

@jnoordsij
Copy link
Contributor

@Hyzual I made https://github.com/jnoordsij/stylelint-config-recommended-scss/tree/allow-default-annotation to do this. Feel free to cherry-pick it and finish this PR.

@jnoordsij
Copy link
Contributor

Note that in stylelint-scss/stylelint-scss#666 there's a request for a custom scss version of this rule. If that lands, it would allow for more customised handling and would remove the need for manual configuration.

@Hyzual
Copy link
Contributor Author

Hyzual commented Oct 24, 2022

@jnoordsij Thanks! I've pulled your branch and force-pushed this PR
@kristerkari the PR is rebased on top of master, @jnoordsij added examples to the valid.scss file :)

Copy link
Collaborator

@kristerkari kristerkari left a comment

Choose a reason for hiding this comment

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

Looks really good now 👍

Thanks @Hyzual @jnoordsij !

@kristerkari kristerkari merged commit d62f898 into stylelint-scss:master Oct 24, 2022
@Hyzual Hyzual deleted the allow-default-annotation branch October 26, 2022 07:02
nickcharlton added a commit to thoughtbot/administrate that referenced this pull request Feb 2, 2024
nickcharlton added a commit to thoughtbot/stylelint-config that referenced this pull request Feb 2, 2024
In addition to being valid SCSS, we often use `!default` in places like
`variables.scss`.

stylelint-scss/stylelint-config-recommended-scss#131
stylelint-scss/stylelint-config-recommended-scss#132
nickcharlton added a commit to thoughtbot/stylelint-config that referenced this pull request Feb 2, 2024
In addition to being valid SCSS, we often use `!default` in places like
`variables.scss`.

stylelint-scss/stylelint-config-recommended-scss#131
stylelint-scss/stylelint-config-recommended-scss#132
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

annotation-no-unknown should allow !default
3 participants