-
Notifications
You must be signed in to change notification settings - Fork 162
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
chore: separate schema generation #2886
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
} | ||
|
||
var genTypesSchemaCmd = &cobra.Command{ | ||
Use: "gen-types-schema", |
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 didn't build a replacement for this as I don't think it's needed. I'm not sure what the original use case was for adding this, though I don't think it makes sense to have an internal command generating a schema for types users shouldn't be creating.
Good jobs getting this done. A couple of comments. Is the docs changes related to these changes or are we fixing this because we found out it was wrong during this work? We wont remove any dependency in the Zarf CLI in this change. Just making sure because the description makes it seem like we are reducing the amount of dependencies. The last thing is that I think this code should go in a I would like @schristoff input on this type of project structure. |
Signed-off-by: Austin Abro <[email protected]>
It's related to this work. Essentially we're copying the alpha schema to a different folder. I'm not deleting the original zarf.schema.json since many users rely on it. However once we're in v1 and users no longer need that original zarf.schema.json I'm thinking we can move all the schemas to the tools folder
Yeah my description was confusing, fixed it.
I would agree |
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
✅ Deploy Preview for zarf-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Description
This moves schema generation to it's own folder rather than being a part of the Zarf binary. This moves the schema's to their own folder to help keep the repo clean as the number of schemas will grow as we introduce schema versions. However this does not delete the
zarf.schema.json
at the root of the repo as we've pointed users to that file in the docs to get schema validation in VS code.This doesn't remove the invopop/jsonschema from the Zarf CLI, however there is only one place left it is used. Likely this can be removed in a future PR and replaced with runtime validation.
zarf/src/api/v1beta1/component.go
Line 318 in 8100c69
Relates to #2788
Checklist before merging