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

Redefined lexical constraint on 'token' datatype #183

Merged
merged 14 commits into from
Apr 15, 2022

Conversation

wendellpiez
Copy link
Collaborator

Addressing #181

Committer Notes

In this PR, references to upper-Unicode characters that break at least one processor in the XSD that break at least one processor (in the token datatype definition) have been removed.

This is a change only in the XSD syntax not in any definitions. The datatype (mirroring NCName) is not actually being relaxed here, since the exclusion we are removing tests out as inoperative. The offending characters (starting with \u10000) are not in the sets from which they are being excluded (XML name character \c and initial name character \i). (We should have detected this discrepancy earlier, but didn't.)

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you squashed any non-relevant commits and commit messages? [instructions]
  • Do all automated CI/CD checks pass?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?
  • Have you updated all website](https://pages.nist.gov/metaschema) and readme documentation affected by the changes you made? Changes to the website can be made in the website/content directory of your branch.

aj-stein-nist
aj-stein-nist previously approved these changes Mar 5, 2022
Copy link
Collaborator

@aj-stein-nist aj-stein-nist left a comment

Choose a reason for hiding this comment

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

Hey @wendellpiez, @david-waltermire-nist asked I review this pull request. I tested it on my workstation and ran the pipeline locally with the code as configured in usnistgov/OSCAL#1161 (the GitHub Actions do not run the metaschema-artifacts job since it is just a submodule update). Given that we tested these exact regexes to not fail in the C# harness here in Issue1127Example repo Schema12 test, I removed draft status from this PR and definitely approve moving it forward. 🚢

aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this pull request Mar 9, 2022
…moving references to upper-Unicode characters that break at least one processor - this is not actually relaxed, since the exclusion tests out as inoperative (the offending characters are not in the sets from which they are being excluded)
…e no upper-Unicode characters in the schema to be concerned about.
@wendellpiez
Copy link
Collaborator Author

wendellpiez commented Mar 23, 2022

NB: the scope of this PR has changed. It now includes work addressing usnistgov/OSCAL#1186, which supersedes the datatype adjustments originally planned and implemented.

@wendellpiez
Copy link
Collaborator Author

@aj-stein-nist the adjusted schema generators here support the new improved datatype definitions while also being backward compatible - feel free to give them a go (JSON Schema and XSD generators should update).

@wendellpiez
Copy link
Collaborator Author

wendellpiez commented Apr 1, 2022

Code in this commit now produces decent-looking XSD and JSON Schemas from their respective pipeline, with updated datatypes.

We should address datatype alignment again after new Metaschema schemas and infrastructure are deployed. We can then remove use of old datatypes in the metaschema sources, and tighten all this back.

However, this is ready for updating data type support for 1.0.3 (assuming it integrates and passes downstream checks).

Copy link
Collaborator

@david-waltermire david-waltermire left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants