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

Fix generating invalid YAML files #282

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

Dunedan
Copy link
Contributor

@Dunedan Dunedan commented Oct 4, 2024

YAML 1.0 demands that the separator between a YAML directive and its value is a colon. This wasn't the case for the YAML files generated by spirv-reflect, as it used a space as separator instead. This resulted in tools parsing YAML stricly not being able to process the YAML files generated by spirv-reflect.

Newer YAML versions demand a space as separator between YAML directive and its value instead.

To fix this, this commit increases the YAML version to 1.1. This is done instead of replacing the separator to improve compatibility with tooling, as many tools don't support YAML 1.0 and require YAML 1.1 as minimal version (see the list of tools on https://yaml.org/).

YAML 1.0 demands that the separator between a YAML directive and its
value is a colon. This wasn't the case for the YAML files generated by
spirv-reflect, as it used a space as separator instead. This resulted in
tools parsing YAML stricly not being able to process the YAML files
generated by spirv-reflect.

Newer YAML versions demand a space as separator between YAML directive
and its value instead.

To fix this, this commit increases the YAML version to 1.1. This is done
instead of replacing the separator to improve compatibility with
tooling, as many tools don't support YAML 1.0 and require YAML 1.1 as
minimal version.
@CLAassistant
Copy link

CLAassistant commented Oct 4, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@spencer-lunarg spencer-lunarg left a comment

Choose a reason for hiding this comment

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

@chaoticbob this seems fine with me, are you aware of any automated tools that were consuming these YAML that would break from this

@spencer-lunarg
Copy link
Contributor

been a week, going to merge this since in theory it was already broken pretending to be 1.0 doing 1.1 things

@spencer-lunarg spencer-lunarg merged commit f7b977a into KhronosGroup:main Oct 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants