-
Notifications
You must be signed in to change notification settings - Fork 653
refactor(rome_service): move globals
, refactor rule
configuration
#2907
Conversation
Deploying with
|
Latest commit: |
9dd22b2
|
Status: | ✅ Deploy successful! |
Preview URL: | https://10ce985a.tools-8rn.pages.dev |
Branch Preview URL: | https://refactor-linter-configuratio.tools-8rn.pages.dev |
Parser conformance results on ubuntu-latestjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
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.
globals
{
"root": true,
"javascript": {
"formatter": {
"quoteStyle": "double"
},
"linter": {
"globals": ["$"]
}
}
}
- How do you intend to represent globals for other languages like HTML (global web component names), or CSS (global styles)?
- I would move
globals
out oflinters
because knowing the globals is also something that is useful in the context of modification where globals should never be mangled. Tough, closure uses explicitextern
files that also provide type annotations for better results.
Init command
What's the migration strategy if Rome's default change between two versions? Do users have to explicitly upgrade their configuration or should new defaults be applied automatically?
Let's say we release the linter and enable it by default. Do now all Rome users have to enable the linter manually to make use of it?
The configuration of
Done. It makes total sense.
I am not sure I fully understand the question. But for CSS I would see something like
Migrations for configuration files was in plan for Rome classic. There are two ideas on the table, which can leave together. Having a The other idea is to map transformations of certain sections of the configuration from one version to another. For example, let's say we decided to rename |
c87bd17
to
cd670dd
Compare
Summary
This PR does four things
globals
fieldThis field has been moved inside the JavaScript language, it is now like this
rules
settingsNow the rules can be configured in two different way. The old one and a new one, in case we will need to configure some rules:
options
is defined as aValue
for now, which means that anything can be assigned to it. I don't have a clear idea how this field will be de-serialized/configured and documented inside our code generation, but at least this data structured gives us an edge where we don't need to apply breaking changes, becauseoptions
is now a wildcard.rome init
commandNow the command will turn off the
linter
by default, as we at our last meeting. I also added more text to the command. This text is exactly the same text that the Rome classic used to show. I didn't change anything, other than some link.Configuration validation
In my last PR I didn't add the attribute
deny_unknown_fields
to all the structs. I was under the impression that the attribute worked for all thestruct
once defined in the main one, but nope! It should work nowTest Plan
Updated the existing cases to match the structure of the new configuration file.
Here's how the
rome init
is shown. Unfortunately the currentmarkup
macro is a bit difficult to tame when it comes to add tabbing and newlines.