-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix broken YAML #74
Fix broken YAML #74
Conversation
Enums can be specified either as an array or as a hash. Hash values are currently ignored when converting to json schema, only the keys are used: So the resulting json schema when using an array and a hash is the same. But the hash variant mean we have a description of each enum key, which can be useful for documentation. A hash with empty values is allowed, it simply means there is no description of the enum keys. But in that case it's cleaner to use an array, as is used in e.g. A0303: values: ['on','off'] So the current sxl.yaml is technically correct, but I think we should either keep using a hash but add an explanation for the on and off enums key, or switch to an array. Since A0303 has the same |
If we don't like hashes with empty values, we can add an error or warning when the converter is run. |
This reverts commit c034942.
Then I prefer |
ok. i've fixed it in the same way for SXL<1.1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unresolved merge conflict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, fixed now
by the way, yaml allows strings without quotes in arrays: [banana,guava] But |
on a side note: our checks do not catch bugs in the yaml, because they don't regenerate the json schema. |
i think this is ready for merging |
A side note: |
It seems on and off are treated as normal strings from YAML 1.2, but the yamllib c lib that is used in both ruby and python does not yet fully support 1.2: yaml/libyaml#20 I prefer to stick to the yaml parsing spec, rather than use flags or patching to modify the behaviour. |
There is a slight error with A0301 where 'on' and 'off' is written as a key value pair, but only the first part exists.
E.g.
But needs to be:
An alternative would be to write it as
[ 'on', 'off' ]
, but that is treated as an array. Both variants exists in the YAML. Perhaps we should only use key/value pairs?