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

Add JSON Schema for package configuration #5370

Conversation

mmurto
Copy link
Contributor

@mmurto mmurto commented May 23, 2022

Add a JSON Schema for package configuration for IDE validation.

Can be tested with VSCode by adding the following in .vscode/settings.json of a workspace containing package configurations and copying the schema to schemas/package-configuration-schema.json of the workspace:

{
  "yaml.schemas": {
    "./schemas/package-configuration-schema.json": [
      "source-artifact.yml",
      "vcs.yml"
    ]
  },
}

@mmurto mmurto requested a review from a team as a code owner May 23, 2022 12:51
Comment on lines +146 to +157
"oneOf": [
{
"required": [
"vcs"
]
},
{
"required": [
"source_artifact_url"
]
}
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this should be correct according to the JSON Schema spec, but vscode-yaml plugin currently does not correctly handle it. The plugin does not produce error if both vcs and source_artifact_url are provided.

@sschuberth
Copy link
Member

LGTM, but maybe @MarcelBochtler can chime in as he's got the most experience here.

Also, can we submit this to a schema registry, too?

@mmurto
Copy link
Contributor Author

mmurto commented May 23, 2022

Also, can we submit this to a schema registry, too?

That would be great!

@MarcelBochtler
Copy link
Member

MarcelBochtler commented May 24, 2022

I tested it and it works great :)

@MarcelBochtler
Copy link
Member

I also hoped that it would be possible to use this package-configuration schema and somehow include it as a reference in the repository-configuration-schema.json.
But as I didn't find a way to do that some months ago, I'd be okay, to duplicate the data.

Signed-off-by: Mikko Murto <mikko.murto@hhpartners.fi>
@mmurto mmurto force-pushed the package-configuration-json-schema branch from 9ccf2ee to 84179f6 Compare May 25, 2022 06:15
@mmurto
Copy link
Contributor Author

mmurto commented May 25, 2022

I also hoped that it would be possible to use this package-configuration schema and somehow include it as a reference in the repository-configuration-schema.json. But as I didn't find a way to do that some months ago, I'd be okay, to duplicate the data.

I haven't tried it myself, but according to this, it should be possible to refer to schema from inside of another.

@codecov
Copy link

codecov bot commented May 25, 2022

Codecov Report

Merging #5370 (84179f6) into main (f6d9ea2) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #5370   +/-   ##
=========================================
  Coverage     72.14%   72.14%           
  Complexity     1982     1982           
=========================================
  Files           264      264           
  Lines         13979    13979           
  Branches       1988     1988           
=========================================
  Hits          10085    10085           
  Misses         2832     2832           
  Partials       1062     1062           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6d9ea2...84179f6. Read the comment docs.

@sschuberth sschuberth requested a review from MarcelBochtler May 25, 2022 06:49
@sschuberth sschuberth merged commit 1607a8c into oss-review-toolkit:main May 25, 2022
@mmurto mmurto deleted the package-configuration-json-schema branch May 25, 2022 10:43
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.

3 participants