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

feat(biome_css_analyzer): initilize css linter infra #2111

Merged
merged 10 commits into from
Mar 19, 2024

Conversation

togami2864
Copy link
Contributor

@togami2864 togami2864 commented Mar 16, 2024

Summary

I've experimentally created a new task called just new-css-lintrule and initialized the ColorNoInvalidHex NoColorInvalidHex CSS linter rule as an example. I'll work on the lintdoc, the website, and rustdoc in another PR later.

Since this is an experiment, feel free to comment on anything about the direction, even if it's not directly related to the code:)

Test Plan

Run the new task on my local machine and check the files.

@github-actions github-actions bot added A-Project Area: project A-Tooling Area: internal tools L-CSS Language: CSS A-Diagnostic Area: diagnostocis labels Mar 16, 2024
Copy link

netlify bot commented Mar 16, 2024

Deploy Preview for biomejs ready!

Name Link
🔨 Latest commit 962095e
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/65f7b7a5306d720008128689
😎 Deploy Preview https://deploy-preview-2111--biomejs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 99 (🟢 up 3 from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 93 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@togami2864 togami2864 requested review from a team March 16, 2024 12:17
/// }
/// ```
///
pub ColorNoInvalidHex {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've adjusted Stylelint anyway. Should we consider enforcing naming conventions similar to js linters?
https:/biomejs/biome/blob/main/crates/biome_analyze/CONTRIBUTING.md#choose-a-name

Copy link
Member

@ematipico ematipico Mar 16, 2024

Choose a reason for hiding this comment

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

At the beginning the convention made sense because we had few query types, but now I don't think it makes sense anymore. I think we should just kill the convention and put all the rules in the same folder. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally agree that idea since that convention confuses some contributors sometimes. I often see that the rule uses Ast is putted in the /semantic_analyzer...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ematipico What about lint rule name convention? Should we adhere to the rule names in Stylelint?

Copy link
Member

Choose a reason for hiding this comment

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

Rule names should adhere to our convention :)

Copy link

codspeed-hq bot commented Mar 16, 2024

CodSpeed Performance Report

Merging #2111 will not alter performance

Comparing togami2864:feat/css-linter-infra (962095e) with main (984626d)

Summary

✅ 93 untouched benchmarks

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you for taking the lead! That's an amazing step forward :)

Whenever you can, can you update this table and put the ⌛ emoji for the CSS linting?

I am going to merge this PR so you can continue your works

@ematipico ematipico merged commit 98d53ae into biomejs:main Mar 19, 2024
17 checks passed
@Conaclos Conaclos added the A-Changelog Area: changelog label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Diagnostic Area: diagnostocis A-Project Area: project A-Tooling Area: internal tools L-CSS Language: CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants