Fix schema and it's validation#30
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
This comment was marked as resolved.
This comment was marked as resolved.
|
@bennettgoble and @WolfGangS Could you take a quick look at this while I'm at it since you contributed the original code? |
This comment was marked as resolved.
This comment was marked as resolved.
|
@WolfGangS Thanks for the feedback.
With regard to not using third-party actions, GitHub supports adding action code to |
I think a local method for a contributor to validate their work before committing is needed anyway, if something vaguely like what I suggest was added, then the action wouldn't really be more complex than the current step in validate that runs - name: Check if definitions file matches schema
run: |
# Generate syntax to a temporary file to verify it's valid
gen-lsl-definitions ./lsl_definitions.yaml syntax /tmp/syntax_output.llsd
# Check that the output file was created and is not empty
if [ ! -s /tmp/syntax_output.llsd ]; then
echo "Error: Syntax generation produced empty output"
exit 1
fi
echo "✓ Syntax generation successful and produced non-empty output"One could just add - name: Check if syntax generation produces valid output
run: |
# Run command to check the lsl_definitions against the schema file, to verify it's valid
validate-schema
echo "✓ Definitions file matches schema" |
|
I have addressed all comments above. Haven't tested myself locally, yet. But everything is ready to go. I'll switch to "ready for review" once I've tested locally, but feel free to do it yourself and provide feedback in the meantime. |
HaroldCindy
left a comment
There was a problem hiding this comment.
Seems largely reasonable to me, thanks! Just a couple questions.
|
Looks like CI is failing, think the command name in the workflow def needs updating |
address pull request comment on secondlife#33
|
@WolfGangS I had to move |
HaroldCindy
left a comment
There was a problem hiding this comment.
Generally looks pretty good, just a couple of nits!
This commit ...
yamale's so we canbool-semanticsfor function parameters #33, addresses changes proposed there by including them in the new format used here