Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Validate the value set #9

Closed
wants to merge 6 commits into from
Closed

Conversation

skounis
Copy link
Contributor

@skounis skounis commented Jul 15, 2021

Setup an action that validates the valuesets.

It fetches the valueset.json and validates all the .json files

@gabywh
Copy link
Contributor

gabywh commented Oct 12, 2021

  1. No need to fetch: valueset.json should now be included in this repo.
  2. However, this does NOT validate the valueset as such, only the JSON structure.
  3. If you want to simply validate the JSON structure, then sure use e.g. ajv with valueset.json as schema defn. Any decent text editor would, however, almost certainly have flagged any JSON structural inconsistencies, so the value of this step is debatable - particularly as it may generate a false sense of security with people thinking it is all good when the reality is that it only checks if you have matched the { } plus a little more...

@skounis
Copy link
Contributor Author

skounis commented Oct 12, 2021

I'm a bit confused. Why we keep the valueset.json which is the schema in this repo instead of keeping it in the repository where we maintain all the schemas? No that technically makes any difference. We should however keep this file in just one place.

What it validates is if the JSON is well structured and also if the required properties are there and spelled correctly. A developer could do this in the editor but we do not have any control on what will be committed. We all do mistakes. For example:

Wrongly spelled property
image

Missing property
image

@gabywh
Copy link
Contributor

gabywh commented Oct 13, 2021 via email

@dslmeinte
Copy link
Contributor

It seems this change is also part of PR #26 : can we just look at that PR, or do you want to merge these changes (“automically”) in stages, @skounis ?

@ryanbnl ryanbnl closed this Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants