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

feat(rome_service): add $schema to configuration #3738

Merged
merged 6 commits into from
Nov 21, 2022

Conversation

ematipico
Copy link
Contributor

Summary

Closes #3690

  • added a new $schema property to the configuration
  • updated the codegen-configuration and codegen-schema aliases to store the first paragraph of the documentation of the rules
  • updated the rome package to publish a new file called configuration_schema.json
  • updated the Default implementation of Configuration, with the $schema value that points to ./node_modules/rome/configuration_schema.json.
    This change is very biased. I thought that it could be a nice DX improvement to have it as part of the rome init command. The path is hard coded, I checked on the internet and it seems that all local paths use unix paths, it don't seem to be OS dependent.

Test Plan

I updated the existing CLI tests. The CI should pass for code generated files.

@ematipico ematipico requested a review from a team November 15, 2022 10:17
@netlify
Copy link

netlify bot commented Nov 15, 2022

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 99b92b5
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/637b43197061a9000a325a63
😎 Deploy Preview https://deploy-preview-3738--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 added the A-Project Area: project configuration and loading label Nov 15, 2022
@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44936 44936 0
Failed 943 943 0
Panics 0 0 0
Coverage 97.94% 97.94% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 1621 1621 0
Failed 4325 4325 0
Panics 0 0 0
Coverage 27.26% 27.26% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12397 12397 0
Failed 3860 3860 0
Panics 0 0 0
Coverage 76.26% 76.26% 0.00%

@ematipico ematipico requested a review from leops November 15, 2022 16:19
@@ -0,0 +1,1230 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add this file (and its copy under editors/vscode) to the list of auto-generated files in .gitattributes

@@ -226,6 +231,16 @@ pub fn create_config(
}
})?;

// we now check if rome is installed inside `node_modules` and if so, we
let schema_path = PathBuf::from("./")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Use Path::new("./node_modules/rome/configuration_schema.json") to avoid allocating a PathBuf object (and also avoid using platform-specific path separators, since I think join would use \ on Windows and cause the snapshot tests to fail)

@ematipico ematipico merged commit 3bcf54c into main Nov 21, 2022
@ematipico ematipico deleted the feature/json-schema-field branch November 21, 2022 11:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Project Area: project configuration and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📎 Allow "$schema" in rome.json
3 participants