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

Use friendly unique names for all Khronos extension schemas #2314

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

aaronfranke
Copy link
Contributor

This PR improves the schema titles. This is mostly cosmetic, but it may have practical benefits, such as improved human and machine parsability, and it gives us the ability to uniquely identify a schema file from its title alone.

When making this PR, I used the following rules:

  • All titles should be in Title Case where words start with a capital letter except for stylized glTF.
  • All extension schemas should include the extension name at the start. For example, all KHR_lights_punctual schemas should start the title with "KHR_lights_punctual".
    • Previously the light properties schema was just titled "light". This is not unique enough to be helpful without the context of knowing you are dealing with KHR_lights_punctual.
  • Schemas that extend a specific part of glTF should specify that part in the format "glTF <something> Extension".
    • If the extension is applied on the base document, use the text "glTF Document Extension".
    • Note that KHR_xmp_json_ld.schema.json can be applied anywhere, so it does not apply to a specific part, so I kept it as just "glTF Extension".

In this PR, I only touched the Khronos extensions in the 2.0 folder.

@javagl
Copy link
Contributor

javagl commented Aug 23, 2023

While I agree to the goals of this improvement, one has to be careful: If I remember correctly, wetzel (the tool that is used for generating the schema reference) uses the title property in a ... very special way, namely as a sort of "identifier" for the types that are defined.

This is just a hint for now: I don't know how actively wetzel is used for generating the MD of extensions. I think it should at least be possible to do this, but on top of that, I don't know whether the changes here might cause wetzel to choke on the schema.

@emackey Maybe you know from the tip of your head whether these changes could be "critical" or cause wetzel to fail for things for which it previously worked?
(Otherwise, I could give it a try with the old and new state, but ... would have to allocate some time for that...)

Copy link
Member

@emackey emackey left a comment

Choose a reason for hiding this comment

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

Thanks for the ping @javagl. I'm fine with the changes here. Currently we are not running wetzel on any extension schemas, only core, so checking wetzel compatibility is out-of-scope for this PR. It looks like the changes here add consistency and clarification. LGTM.

Copy link
Member

@lexaknyazev lexaknyazev left a comment

Choose a reason for hiding this comment

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

LGTM. Would need to be rebased on #2313.

@emackey
Copy link
Member

emackey commented Aug 28, 2023

Would need to be rebased on #2313.

Looks like #2313 needs more work. Let's merge this one first.

@emackey emackey merged commit 1bde276 into KhronosGroup:main Aug 28, 2023
1 check passed
@aaronfranke aaronfranke deleted the schema-titles branch August 28, 2023 15:15
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.

4 participants