-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Scope selector-class-pattern stylelint rule to web stylesheets #29881
Scope selector-class-pattern stylelint rule to web stylesheets #29881
Conversation
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
Native stylesheets currently do not adhere to the same BEM selector patterns as web stylesheets. This introduces a custom stylelint plugin to exclude native stylesheets from the selector-class-pattern rule.
47e8cf3
to
0dfdd0c
Compare
Aside from the pre-existing linting failures, the CI check failures in this PR appear to be resolved in #29800. Once |
What's the difference between this PR and #28988 other than there are no code comments in native In both cases, there is some further work necessary to make the linter pass. The remaining violations need to be fixed wherever applicable. I think there are some exceptions:
|
@gziolo that is the only difference. Native stylesheets and avoiding unnecessary linting noise is the sole focus of this PR. If we decide we do want native stylesheets to adhere to BEM, this PR provides no value. If we decide we do not want native stylesheets to adhere to BEM, this PR allows native developers to avoid the need to remember to add an inline comment to disable the web-specific rule for new stylesheets. This PR avoids the following scenario:
Avoiding that unexpected, unnecessary friction is the focus of this PR.
Absolutely. This PR was solely focused on native stylesheets and iterating on the great progress #28988 made on that subject. I was not attempting to solve all existing linting failures in this PR as I figured that was the scope of #28954. This PR is requesting to merge into
I perceive Which native file has a filename that does not include I hope this provides insight into my thinking. Thanks for the feedback! |
See: https://github.com/WordPress/gutenberg/pull/29881/checks?check_run_id=2118076618
Yes, it's for the web. It isn't vendor code anymore since we maintain a fork of the original library. So the idea is to merge those changes to the branch opened by @rafaelgalani and fix all the remaining issues there, right? |
@gziolo ah, yes. Thank you. 😊 I am happy to investigate resolving that lint error in this PR since it is related to native stylesheets.
Alright. I suppose then it may make most sense to update the forked copy to match the BEM selector patterns we desire to use. I would consider that a part of the scope of #28954.
Yes, if the team believes this PR is a sound approach, then I'd recommend we merge this PR into #28954 and resolve the remaining, non-native-stylesheet-related issues there. |
All stylesheet files that do not have the native suffix are linted with web-specific selector rules. Adding the suffix disables those irrelevant rules for the native stylesheet.
Sorry for taking so long to respond. |
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.
Let's merge this PR into the active branch from @rafaelgalani and continue there (we still need to fix some violations for the web. Nice work @dcalhoun 👍🏻
Description
Native stylesheets currently do not adhere to the same BEM selector
patterns as web stylesheets. This introduces a custom stylelint plugin
to exclude native stylesheets from the selector-class-pattern rule.
Supersedes #28988.
How has this been tested?
Ran
npm run lint-css
.Types of changes
Bug fix
Checklist: