+
Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_service): add all configuration to linter #4258

Merged
merged 8 commits into from
Mar 5, 2023

Conversation

ematipico
Copy link
Contributor

Summary

This PR adds a new configuration to the "linter" section called "all".

This configuration allows users to enable ALL rules or ALL rules for the given group.

recommended and all can't be true simultaneously; that's why I created a specific validation just for these cases.

As for choosing the rules, the logic first checks if rule is Some() and if so, enabled/disabled rules are updated. If all is None, we fallback to check the recommended flag (with its defaults).

This option will be of great help, because it would allow us to use the playground and enable ALL rules without updating the TS code.

Test Plan

I created new test cases to ensure we selected the correct groups.
I also locally checked the playground to see if ALL rules are enabled.

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Mar 4, 2023

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit cb7e008
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/640465dc858d050008cc8ad6
😎 Deploy Preview https://deploy-preview-4258--docs-rometools.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@ematipico ematipico force-pushed the feature/linter-all-rules branch 2 times, most recently from c0ac510 to e9532ca Compare March 4, 2023 18:23
@github-actions
Copy link

github-actions bot commented Mar 4, 2023

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 48647 48647 0
Passed 47582 47582 0
Failed 1065 1065 0
Panics 0 0 0
Coverage 97.81% 97.81% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6093 6093 0
Passed 1783 1783 0
Failed 4310 4310 0
Panics 0 0 0
Coverage 29.26% 29.26% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 568 568 0
Failed 71 71 0
Panics 0 0 0
Coverage 88.89% 88.89% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16740 16740 0
Passed 12815 12815 0
Failed 3925 3925 0
Panics 0 0 0
Coverage 76.55% 76.55% 0.00%

@ematipico ematipico force-pushed the feature/linter-all-rules branch from 7dbe4c6 to 0b62f2e Compare March 4, 2023 23:51
@@ -45,11 +49,19 @@ impl CliSnapshot {
let mut content = String::new();

if let Some(configuration) = &self.configuration {
let parsed = parse_json(&redact_snapshot(configuration));
Copy link
Contributor

Choose a reason for hiding this comment

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

It's so cool that we can use Rome json parse))))

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 completely agree! I think it's a great way to dogfood Rome itself!

@ematipico ematipico force-pushed the feature/linter-all-rules branch from fafa349 to cb7e008 Compare March 5, 2023 09:50
@ematipico ematipico merged commit 9cedc42 into main Mar 5, 2023
@ematipico ematipico deleted the feature/linter-all-rules branch March 5, 2023 09:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载