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

Nonuniform indentation in metaschemas #1183

Open
guyzyl opened this issue Mar 17, 2022 · 6 comments
Open

Nonuniform indentation in metaschemas #1183

guyzyl opened this issue Mar 17, 2022 · 6 comments
Labels
Aged A label for issues older than 2023-01-01 Discussion Needed This issues needs to be reviewed by the OSCAL development team. question

Comments

@guyzyl
Copy link
Contributor

guyzyl commented Mar 17, 2022

I recently noticed that there isn't an indentation convention/uniformity amongst the metaschema files in src/metaschema.
The indentation sizes that exist (all in # of spaces) are:

  • 2
  • 3
  • 4
  • 6
  • 8

This doesn't hurt functionality at all, just isn't very conventional for a code repo to have different indentations for same file types.

Are you open to a PR fixing all indentations to a single convention? My preference is towards 4 spaces (let me know if you think otherwise).


Side note - it might a good idea to consider using an XML linting utility during the CI/CD process, but that's the topic for another issue.
guyzyl added a commit to guyzyl/OSCAL that referenced this issue Mar 17, 2022
This was done in order to create a uniformal indentation between the metaschemas as fully described in usnistgov#1183 which this commit fixes.
@wendellpiez
Copy link
Contributor

wendellpiez commented Mar 22, 2022

(Discussed with @david-waltermire-nist and @aj-stein-nist March 22.) A one-time fix will unfortunately not defend against slippage.

There is a variety of different content formats in the OSCAL repo, including JSON, YAML, Markdown and XML. We need to normalize whitespace styles across repositories, for reasons including supporting useful differencing. For a given contribution, we need a way to ensure that the contribution conforms to (house) whitespace rules.

Also, we want contributors to be able to edit repo content using whatever whitespace style they like best, avoiding imposing whitespace normalization rules during development. Instead, we would like the rules to be enforced only when the PR is made.

The repository CI/CD needs a way to check and enforce these normalization rules. Also when making a PR, devs need tooling to reformat whitespace easily, based on the imposed normalization requirements.

To support this, we need tools that can perform both the normalization and the rules enforcement. We would like input from the community on how to address this in JSON, YAML, XML, and Markdown.

Any suggestions on how this is done?

@david-waltermire david-waltermire added the Lunch with the Devs Issues to discuss during the bi-weekly Lunch with the Devs call label Mar 22, 2022
@david-waltermire
Copy link
Contributor

Raised this on the 3/24/2022 Lunch with the Devs call. We didn't get any feedback for or against working on whitespace normalization. I asked the community to provide feedback on this issue after the meeting.

@david-waltermire david-waltermire added Discussion Needed This issues needs to be reviewed by the OSCAL development team. and removed Lunch with the Devs Issues to discuss during the bi-weekly Lunch with the Devs call labels Mar 24, 2022
@guyzyl
Copy link
Contributor Author

guyzyl commented Mar 24, 2022

@david-waltermire-nist unfortunately I wasn't able to make it to today's lunch with devs and voice my opinion, but it is the one presented in this issue (surprisingly as I'm the one who created it 😅).

@GaryGapinski
Copy link

Just an FYI.

As an experiment, I created a large (100Mb) XML document with no line feeds or indentation and used the following transform to serialize it with Saxon HE 11.1. The output was passable but not quite what I would have expected (and not the style I prefer).

I will also note that https://www.w3.org/TR/xml-c14n11/ is unlikely to be acceptable (as a means by which routine code maintenance and git diffs are tidy).

<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
    xmlns:xs="http://www.w3.org/2001/XMLSchema"
    xmlns:math="http://www.w3.org/2005/xpath-functions/math"
    exclude-result-prefixes="xs math"
    version="3.0">
    <xsl:output method="xml" indent="true"/>
    <xsl:mode on-no-match="deep-copy"/>
</xsl:stylesheet>

@Arminta-Jenkins-NIST
Copy link
Contributor

At the 11/16 Triage Meeting: This issue is insignificant functionally but GitHub can misinterpret the white space. NIST team agreed to defer to @wendellpiez and discuss again on Nov 30th.

@wendellpiez
Copy link
Contributor

Is the problem noise in diffing, or (just) unsightly display when looking at the metaschema source?

In any case there are a couple of mitigations possible, with the easiest very much depending on what XML-wrangling tools you are comfortable with. I'm happy to discuss approaches here.

The comprehensive approach hinted at in the discussion thread is probably out of scope for now. Although we can discuss what it would be. (But it is not an OSCAL solution, it is an all-Metaschema solution that would require OSCAL implementation. I suggest an OSCAL solution.)

For the next person touching the metaschemas to do some whitespace fixup -- this is definitely doable, particularly using the right tool.

@Arminta-Jenkins-NIST one possibility is to bundle this Issue with the next Issue that touches Metaschema source (maybe @Compton-NIST can help determine what that is?), and I can provide guidance and assistance with the task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aged A label for issues older than 2023-01-01 Discussion Needed This issues needs to be reviewed by the OSCAL development team. question
Projects
Status: Todo
Development

No branches or pull requests

6 participants