-
-
Notifications
You must be signed in to change notification settings - Fork 481
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(biome_css_analyze): implement noDescendingSpecificity
#4097
feat(biome_css_analyze): implement noDescendingSpecificity
#4097
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.
Since this rule takes the entire file into account, I think it would be better to split these tests into multiple files.
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 it would be better that way as you said, so I split it at da0b219
CodSpeed Performance ReportMerging #4097 will degrade performances by 6.65%Comparing Summary
Benchmarks breakdown
|
crates/biome_css_analyze/src/lint/nursery/no_descending_specificity.rs
Outdated
Show resolved
Hide resolved
Thank you for your review. |
crates/biome_css_analyze/src/lint/nursery/no_descending_specificity.rs
Outdated
Show resolved
Hide resolved
crates/biome_css_analyze/src/lint/nursery/no_descending_specificity.rs
Outdated
Show resolved
Hide resolved
// | ||
// Read our guidelines to write great diagnostics: | ||
// https://docs.rs/biome_analyze/latest/biome_analyze/#what-a-rule-should-say-to-the-user | ||
// |
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.
Please remove this comment
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.
Fixed at 0dbdbd1
crates/biome_css_analyze/src/lint/nursery/no_descending_specificity.rs
Outdated
Show resolved
Hide resolved
Thank you for your review. Also, I noticed that the performance test in the CI is failing with my PR. But I'm not sure where to make the necessary changes as I lack knowledge. Could you please help me with this? |
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.
Given that the rule is applied to almost all selectors, I thought this regression was acceptable. Or it's ok to address it in a separate PR.
@chansuke @ematipico @dyc3 any thoughts?
Yeah, the regression seems acceptable to me too! |
Summary
Implement
noDescendingSpecificity
and the specificity calculationclose #2810
Test Plan
Added tests and snapshots