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

Fixing typos/syntax errors #520

Merged
merged 1 commit into from
Oct 21, 2023
Merged

Fixing typos/syntax errors #520

merged 1 commit into from
Oct 21, 2023

Conversation

zvr
Copy link
Member

@zvr zvr commented Oct 20, 2023

This fixes various issues that the (new) spec-parser points out.

Among them:

  • mismatches between "name" property and file name and title
  • vocabularies entries not in lowerCamelCase
  • missing sections ("Summary" in most/all cases)
  • empty sections ("Properties" in most/all cases)
  • ... and probably some others I don't remember

@zvr zvr requested review from kestewart and goneall October 20, 2023 11:48
@zvr zvr added this to the 3.0-rc2 milestone Oct 20, 2023
Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

See comment - suggest moving one of the changes to a separate PR unless it causes the spec-parser to fail.

Copy link
Member

Choose a reason for hiding this comment

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

This change is consistent with the naming conventions of SPDX, however, the security profile is attempting to have the resultant JSON Schema match the CVSS schema and this change will make them different.

Since there is a direct mapping, I'm OK with using the SPDX conventions, but @jeff-schutt @tsteenbe @rnjudge should also review

I would move this into a separate PR to give the security team time to review. If the current state causes the spec-parser to fail, then I would suggest merging it in and adding a new issue to discuss the incompatibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is about entries for a vocabulary, which are automatically converted to IRIs of individuals in RDF.

The process of generating the JSON Schema already does many transformations, as it's not a 1:1 correspondence. If the requirement is to have them appear as upper-case strings, this is easily done at that stage.

Copy link
Member

Choose a reason for hiding this comment

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

The process of generating the JSON Schema already does many transformations, as it's not a 1:1 correspondence. If the requirement is to have them appear as upper-case strings, this is easily done at that stage.

Based on the above, it sounds like we can go ahead and move forward with this PR pending one more review.

I'll leave it up to the security profile group to decide if it is important to maintain uppercase.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, let's apply this, and then ask them to do a pass.

Copy link
Contributor

@kestewart kestewart left a comment

Choose a reason for hiding this comment

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

Apply these changes, but then let's get the security team to review.

They'll need to update their examples.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, let's apply this, and then ask them to do a pass.

@kestewart kestewart merged commit 6b45815 into spdx:main Oct 21, 2023
@zvr zvr deleted the fix-syntax branch October 21, 2023 09:42
@bact bact added the typo Typos, minor spelling errors, or minor incorrect word usage label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typo Typos, minor spelling errors, or minor incorrect word usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants