-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Prepare 23.0.0 #198
Prepare 23.0.0 #198
Conversation
[Google's CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html#CSS_Formatting_Rules), [Airbnb's Styleguide](https://github.com/airbnb/css#css), and [@mdo's Code Guide](https://codeguide.co/#css). | ||
|
||
It favours flexibility over strictness for things like multi-line lists and single-line rulesets, and tries to avoid potentially divisive rules. | ||
|
||
Use it as is or as a foundation for your own config. |
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.
We can take this second line out as we encourage users (lower down the README) to extend the config with more rules that limit language features, e.g. unit-allowed-list
. These are some of the most useful, unique and powerful rules in stylelint. We should encourage users to investigate them. They tend to be specific to teams and projects, so we can't add them to this config.
@@ -100,23 +108,7 @@ npm install stylelint-config-standard --save-dev | |||
|
|||
## Usage | |||
|
|||
If you've installed `stylelint-config-standard` locally within your project, just set your `stylelint` config to: |
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.
We can take out this legacy guidance.
|
||
/** | ||
* Multi-line comment | ||
*/ | ||
|
||
:root { | ||
--brand-red: hsl(5deg 10% 40%); |
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.
We can add an example of modern custom properties.
top: calc(calc(1em * 2) / 3); | ||
top: calc(100% - 2rem); |
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.
We can remove this odd nested calc
.
background: color(rgb(0, 0, 0) lightness(50%)); | ||
font-family: helvetica, "arial black", sans-serif; | ||
background: hsl(20deg 25% 33%); |
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.
We can remove this old color
function.
Incidentally, the upcoming relative color syntax looks great!
- Specify a maximum line length using: | ||
- [`max-line-length`](https://github.com/stylelint/stylelint/blob/master/lib/rules/max-line-length/README.md) | ||
|
||
### Using the config with SugarSS syntax |
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.
We can take this section out as we'll be encouraging users to adopt appropriate shared-configs for their language of choice.
index.js
Outdated
@@ -37,13 +40,16 @@ module.exports = { | |||
ignore: ['after-comment', 'inside-single-line-block'], | |||
}, | |||
], | |||
'custom-media-pattern': /^([a-z][a-z0-9]*)(-[a-z0-9]+)*$/, |
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 think all rules with RegExps would be confusing for non-power user. Because if there is a violation we will show this RegExp and use might not be familiar with it at all.
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.
Good point.
We can use the message
secondary option to make them simpler, e.g.:
Expected custom media name to be kebab-case
It's one of the reasons we have that option.
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.
We could try that.
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.
Done. Each is slightly adapted from the original message in the rule.
Love this, and whilst I am in broad agreement with everything proposed, I've one question: https://www.npmjs.com/package/stylelint-config-standard has 1,337,936 weekly downloads, this is pretty significant (and pretty cool), but when this is released it will be a breaking change... Would it be worth considering that for stylelint v14 these new rule changes are added as warnings, then in the next major release changed to the default error. I'm thinking this might help facilitate non-blocking uptake of the new release for users, this then gives them time to update their CSS before the following major release? |
I'd prefer to have new rules as errors to be consistent with previous major releases on this package. However, I understand your point. What we can do is better emphasise in the README how to turn rules off or change their severity to warnings. I'll amend the pull request to do this. We'll then be consistent in our recommendation that users adopt the |
@ntwb In 6db6439, I've expanded the "Extending the config" section of the README to give explicit examples of turning off and lowering the severity of rules. And I've linked to that section from the migration notes in the CHANGELOG. Tests will pass once #199 is merged and this branch rebased. This pull request is ready for review. When stylelint 14 and recommended config 6 are released we should:
|
Thanks, looks great 💯
I approved and merged this in and updated the branch here in this PR |
We're making changes to get users up and running quickly with comprehensive linting that doesn't leave gaps, e.g. by encouraging users to extend an appropriate shared-config for their choice of styling language.
This pull request is part of those changes. It turns on a handful of rules that'll help new users write modern CSS that looks like something they'd see in the specifications.
For context, our previous approach was to provide the minimal configurations and suggest users build on top of them, going as far as originally emphasising this config and the recommended one in our docs equally. However, I don't think this is helpful for the majority of new users, even those who intend to use stylelint and a pretty-printer like Prettier together, as there are a number of stylistic rules in stylelint and this plugin that are complementary to Prettier, e.g. the
*-empty-line-before
ones. Better to extend a standard config and use*-config-prettier
to turn off conflicting rules.Power users who wish to craft their own config on top of the recommended config can still do so. The changes in this pull request are to provide better linting for non-power users writing CSS.
Ref: stylelint/stylelint#5205