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

Update output to make the root object an overview with results contained inside #1487

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

gregsdennis
Copy link
Member

@gregsdennis gregsdennis commented Feb 8, 2024

This makes progress toward some of what is discussed in #1065.

Basically, we need a place to include metadata about an evaluation. This PR wraps the existing output in a root node that can contains any such metadata.

(As a side effect, it creates a purpose for the root object in the case of the "list" format, which could have just been an array before.)

@gregsdennis gregsdennis requested a review from a team February 8, 2024 23:27
@gregsdennis
Copy link
Member Author

Re-reading through the issue, it seems that dialect should be part of the output unit itself rather than the summary. Still... I think the summary root node is a good addition.

Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Re-reading through the issue, it seems that dialect should be part of the output unit itself rather than the summary. Still... I think the summary root node is a good addition.

I agree that dialect should be part of the output unit. I'm not sure putting it in the summary makes sense. Validation can include references to other schema resources that use a different dialect. So, dialect in the summary is only telling you the dialect of the root schema. Maybe that's ok, but it might be misleading if the evaluation includes multiple dialects.

@gregsdennis
Copy link
Member Author

I'll remove dialect from this PR and do another one for that then.

I know we had a discussion somewhere on having some more "global" details in the root node, though...

Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

You missed a couple places removing dialect, but otherwise, this looks good to me.

output/schema.json Outdated Show resolved Hide resolved
jsonschema-validation-output-machines.md Outdated Show resolved Hide resolved
output/schema.json Outdated Show resolved Hide resolved
gregsdennis and others added 3 commits February 16, 2024 09:50
Co-authored-by: Jason Desrosiers <jdesrosi@gmail.com>
Co-authored-by: Jason Desrosiers <jdesrosi@gmail.com>
Co-authored-by: Jason Desrosiers <jdesrosi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants