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

Improvements to converter readme docs #1027

Merged

Conversation

wendellpiez
Copy link
Contributor

This corrects docs errors reported in #1020.

Potential errors in SSP conversion remain to be determined but will be addressed in usnistgov/metaschema#174.

Committer Notes

There are several ways to run the converters in debug as well as regular configuration. In addition to correcting a manifest error (as reported), the docs now describe these configurations.

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:

n/a

@wendellpiez wendellpiez marked this pull request as ready for review September 28, 2021 13:56
Copy link
Contributor

@ohsh6o ohsh6o left a comment

Choose a reason for hiding this comment

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

Some minor feedback around typos and Saxon version specifications.

@@ -71,7 +71,24 @@ The following example uses **Saxon HE** to convert an OSCAL catalog XML file to
```
java -jar "saxon9he.jar" -xsl:"oscal_catalog_xml-to-json-converter.xsl" -s:"oscal-catalog.xml" -o:"oscal-catalog.json" json-indent=yes
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps change to "saxon10he.jar" or something analogous as Saxon 9.x is now deprecated.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wendellpiez Can you add an issue for upgrading to Saxon 10? Can you reference in that issue that these docs need to be updated?


The Saxon JAR file is named ```saxon9he.jar```. The catalog converter is specified as ```-xsl:"oscal_catalog_xml-to-json-converter.xsl"```, the source catalog XML file is specified as ```-s:"oscal-catalog.xml"```, and the destination catalog JSON file is specified as ```-o:"oscal-catalog.json"```. Paths\names of these files need to be provided based on the location of the files on your computer.
* The Saxon JAR file is named ```saxon9he.jar```.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto here.

xml/README.md Outdated
The Saxon JAR file is named ```saxon9he.jar```. The catalog converter is specified as ```-xsl:"oscal_catalog_json-to-xml-converter.xsl"```, the source catalog JSON file is specified as ```json-file="oscal-catalog.json"```, and the destination catalog XML file is specified as ```-o:"oscal-catalog.xml"```. Paths\names of these files need to be provided based on the location of the files on your computer.
`-it` indicates the initial template (XSLT entry point) should be 'make-xml'.

Paths\names given to other settingsneed to be provided based on the location of the files on your computer:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Paths\names given to other settingsneed to be provided based on the location of the files on your computer:
Paths\names given to other settings need to be provided based on the location of the files on your computer:

xml/README.md Outdated

Paths\names given to other settingsneed to be provided based on the location of the files on your computer:

* The Saxon JAR file is named ```saxon9he.jar```.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto re Saxon9 here.

Copy link
Contributor

@ohsh6o ohsh6o left a comment

Choose a reason for hiding this comment

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

Per discussion with @wendellpiez, I will surface Saxon 9.x versus Saxon 10.x compatibilities elsewhere. The typos and stylistic changes I recommended are now obsolete and have been fixed. I am happy with the improved docs!

@david-waltermire david-waltermire linked an issue Oct 5, 2021 that may be closed by this pull request
@@ -71,7 +71,24 @@ The following example uses **Saxon HE** to convert an OSCAL catalog XML file to
```
java -jar "saxon9he.jar" -xsl:"oscal_catalog_xml-to-json-converter.xsl" -s:"oscal-catalog.xml" -o:"oscal-catalog.json" json-indent=yes
Copy link
Contributor

Choose a reason for hiding this comment

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

@wendellpiez Can you add an issue for upgrading to Saxon 10? Can you reference in that issue that these docs need to be updated?

Copy link
Contributor

@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.

Thanks!

@david-waltermire david-waltermire merged commit 3cbb38e into usnistgov:main Oct 7, 2021
@david-waltermire david-waltermire deleted the issue1020-jsonconverter-bug branch October 7, 2021 18:18
galtm pushed a commit to galtm/OSCAL that referenced this pull request Jan 15, 2022
* Improvements to readme docs for both XML-to-JSON and JSON-to-XML XSLT conversion pathways
galtm pushed a commit to galtm/OSCAL that referenced this pull request Jan 16, 2022
* Improvements to readme docs for both XML-to-JSON and JSON-to-XML XSLT conversion pathways
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.

JSON to XML converter issue
3 participants