-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat: move rule parsing to config-yaml
#6138
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
Conversation
✅ Deploy Preview for continuedev canceled.
|
|
😱 Found 2 issues. Time to roll up your sleeves! 😱 |
@@ -1,10 +1,17 @@ | |||
{ | |||
"name": "@continuedev/config-yaml", | |||
"version": "1.0.88", | |||
"version": "1.0.92", |
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.
reminder to publish this new version once we merge
"exports": { | ||
".": { | ||
"browser": "./dist/browser.js", | ||
"node": "./dist/index.js", | ||
"default": "./dist/index.js" | ||
} | ||
}, |
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 had to add this because I updated unrollAssistant
to include logic from our new config-yaml/markdown
folder. unrollAssistant
is included in registryClient
, which uses node APIs, so we needed a separate entrypoint for browser envs
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.
nice! This is something I didn't know about, and had needed this before
parsedYaml = { | ||
name: rule.name, | ||
version: "1.0.0", | ||
rules: [rule], | ||
}; |
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.
We map markdown rules to yaml rules for now
// TODO: What should be required? | ||
export const rulesJsonSchema = z.object({ | ||
name: z.string(), | ||
version: z.string(), | ||
author: z.string().optional(), | ||
license: z.string().optional(), | ||
rules: z.record(z.string(), z.string()).optional(), | ||
}); | ||
|
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'm not sure if this is where we want to define this, but I'd like to have the types available for use on the hub
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.
So far I'm defining it in a JSON schema here: https://github.com/continuedev/rules-cli/blob/main/internal/validation/schema/rules-schema.json because we'll need it for both CLI and other things. Might not be necessary to define this here. Just making a note to myself here
…ect a header automatically)
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.
Made a few changes to address small bugs, but the design choices here are fine and it's well tested
Consolidates rule parsing functionality by moving it from the core config module to the shared
config-yaml
package.