Closed Abrifq closed 10 months ago
So this format is.. a JSON schema, but serialized as yaml instead of JSON. Correct? I have not heard of this before, but also.. I guess it should work if it parses to an equivalent JS object.
Just to try and get an idea of the use-case for this..
Merging #364 (8e5e99e) into main (23b0afc) will increase coverage by
0.06%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## main #364 +/- ##
==========================================
+ Coverage 95.40% 95.47% +0.06%
==========================================
Files 11 11
Lines 784 795 +11
Branches 176 181 +5
==========================================
+ Hits 748 759 +11
Misses 35 35
Partials 1 1
Files | Coverage Δ | |
---|---|---|
src/cache.js | 97.46% <100.00%> (+0.03%) |
:arrow_up: |
src/io.js | 100.00% <100.00%> (ø) |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
So this format is.. a JSON schema, but serialized as yaml instead of JSON. Correct?
Correct.
* Are there any real world examples of projects using JSON schemas serialized as yaml? * Are there any schemas in this format published on schemastore?
I scraped the schemastore homepage and there is a few, actually.
goss-org/goss project uses goss config schema
Crowdsecurity project uses parser config schema, collection config schema and scenario config schema
Compared to the total amount schemas (711), 4 is really small.
Just to try and get an idea of the use-case for this..
This will only add a way to verify schemas, no more. It's nice to have new line's when you wanna be verbose with your descriptions, and just a personal opinion but yaml just feels more "natural".
Maybe this will give more birth to YAML JSON-Schemas, who knows. This will make verifying with yaml schemas easier.
OK. I think this is a reasonable feature to add.
In order to merge this, I would like us to have at least one unit test in place for this functionality. A test for the success behaviour at the CLI level somewhere in https://github.com/chris48s/v8r/blob/23b0afc66595a3f61868ba5468dd981f335bfba7/src/cli.spec.js#L50 will be fine. The schema we use doesn't need to be very complicated. A yaml version of https://github.com/chris48s/v8r/blob/main/testfiles/schemas/schema.json would be fine (and we can re-use https://github.com/chris48s/v8r/blob/main/testfiles/files/valid.json as the data file to validate). Testing the intricacies of JSON schema is delegated to AJV.
No rush on this. I may not get a chance to review this until next week.
Cheers
Are you up for finishing this off by adding a test? https://github.com/chris48s/v8r/pull/364#issuecomment-1737722936
Yes, just not right now. I'll be travelling for a few days.
gtg fix more lol
I saw it was updated the other day, but I was a bit unclear if it is still WIP or ready for a review. I can have a look over the weekend if it is ready..
It's ready for review.
Make sure to squash/rebase before merging, I had a few rookie mistake commits there 😅
Cool - thanks. I'll put out a 2.1 release with this feature later today
This may require some testing but looks like it works well against simple schemas defined in YAML.
You can use these files as YAML schema tests: https://github.com/Abrifq/yaml-schema-test