Skip to content
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

json schema refactor. #2384

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

json schema refactor. #2384

wants to merge 2 commits into from

Conversation

asadarafat
Copy link

  1. additionalProperties: false is used in more place to forbid extra fields and so we can have strict validation.
  2. Removed minItems from scalar properties (e.g., for string or number). Because thing could go haywire as keyword minItems should be used for array, while the keyword minItems used in type string and number in previous schema
  3. Unified enum values for "mage-pull-policy and restart-policy to avoid duplicates and different case variants.
  4. Factored out complex regex pattern for ports and publish into separate definitions (port-pattern and publish-pattern).
  5. Switched kinds from many hardcoded properties to a single patternProperties rule for flexibility in topology.kinds.
  6. AllOf logic remains intact for kind-conditional type constraints, but ensured they don’t contradict each other.

Your Name added 2 commits January 8, 2025 17:22
…a fields and so we can have strict validation.

2/ Removed "minItems" from scalar properties (e.g., for string or number). Because thing could go haywire as keyword "minItems" should be used for array, while the keyword "minItems used in type string and number.

3/ Unified enum values for "image-pull-policy" and "restart-policy" to avoid duplicates and different case variants.

4/ Factored out complex regex patterns for ports and publish into separate definitions (port-pattern and publish-pattern).

5/ Switched kinds from many hardcoded properties to a single patternProperties rule for flexibility in topology.kinds.

6 / AllOf logic remains intact for kind-conditional type constraints, but ensured they don’t contradict each other.
@@ -0,0 +1,924 @@
{
"$id": "https://containerlab.dev/clab.schema.json",
Copy link
Member

Choose a reason for hiding this comment

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

@asadarafat what is this file for?

Copy link
Author

@asadarafat asadarafat Jan 9, 2025

Choose a reason for hiding this comment

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

@hellt, I assume you are referring to schemas/clab.schema.jsonc. It is essentially the same as clab.schema.json, but with added comments. The .jsonc format is supported by VSCode, allowing for better readability and enhanced documentation within the file.

@hellt
Copy link
Member

hellt commented Jan 31, 2025

this is too big to be merged in one go
we need to raise it in smaller chunks.

Remove the duplicate files, keep the jsonc and add a make target make schema that will create a json file off of it, so that we could keep the comments and have a regular json out of it

@hellt hellt marked this pull request as draft January 31, 2025 22:34
@asadarafat
Copy link
Author

asadarafat commented Jan 31, 2025 via email

@hellt
Copy link
Member

hellt commented Jan 31, 2025

Can you elaborate what do you mean by smaller chunk, perhaps with some example of the chunks?

the changes should be scoped to a smaller amount of existing schema, so it would be easier to test the changes.
Take one improvement out of many and create it as a PR instead of having all of them tucked together as an example

@asadarafat
Copy link
Author

asadarafat commented Jan 31, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants