Skip to content
This repository has been archived by the owner on Aug 5, 2021. It is now read-only.

Review revised schema content and push to production #217

Closed
mattbailey0 opened this issue Jan 5, 2017 · 8 comments
Closed

Review revised schema content and push to production #217

mattbailey0 opened this issue Jan 5, 2017 · 8 comments
Assignees

Comments

@mattbailey0
Copy link
Contributor

mattbailey0 commented Jan 5, 2017

I've reviewed and revised the content in ok-schema-fix and merged into master as discussed. We need a final review from the team and then to deploy the revisions to production.

TODO: change link under the second h2 to the sample json file to point to the production endpoint.

@okamanda
Copy link
Contributor

okamanda commented Jan 6, 2017

@mattbailey0 I just updated the sample code.json file to the one on the master branch:

@michael-balint can you take a look at:

  1. the sample code.json that now lives on the master branch and
  2. the revised markdown for the schema specification that now also lives on the master branch?

and let me know if you think we're missing anything and being inconsistent?

@lukad03 with balint's ok, can you then push to staging?

@michael-balint
Copy link
Contributor

@mattbailey0 - I just updated with my changes. We should also do something with the compliance-metadata-schema directory (probably best to remove it or rename it as being the old 1.0.0 spec).

@ckaran
Copy link

ckaran commented Jan 6, 2017

@okamanda
Copy link
Contributor

okamanda commented Jan 6, 2017 via email

@IanLee1521
Copy link
Contributor

Couple thoughts --

  1. Is the preference currently to update the html version of the schema (https://github.com/presidential-innovation-fellows/code-gov-web/blob/master/src/app/components/policy-guide/docs/compliance/compliance-inventory-code/compliance-inventory-code.template.html) rather than having a Markdown file which is converted? Should proposed changes to the schema now be pull requests against that HTML file?

  2. Perhaps the sample_code.json file should have a multi organization example, e.g.

[
{
    "version":"1.0.1",
    "agency": "GSA",
    "organization": "18F",
    "projects": [ ... ]
},
{
    "version":"1.0.1",
    "agency": "GSA",
    "organization": "TTS",
    "projects": [ ... ]
}
]

One of the reasons for doing that is it highlights a change that needs to be made to the schema. Namely https://github.com/presidential-innovation-fellows/code-gov-web/blob/master/src/app/components/policy-guide/docs/compliance/compliance-inventory-code/compliance-inventory-code.template.html#L20 which says that the code.json file contains a "JSON Object" at the top level, but in reality, it would be better for the top level to be a "JSON Array" of objects.

  1. I think there is a minor bug in the schema HTML (https://github.com/presidential-innovation-fellows/code-gov-web/blob/master/src/app/components/policy-guide/docs/compliance/compliance-inventory-code/compliance-inventory-code.template.html#L37)

Instead of:

      ** For repositories that are only available as government-wide reuse or are closed, pursuant to one of the exemptions, this field will not be required.

It looks like that line should be another nested list:

<ul>
      <li>For repositories that are only available as government-wide reuse or are closed, pursuant to one of the exemptions, this field will not be required.<li>
<ul>

@michael-balint
Copy link
Contributor

michael-balint commented Jan 11, 2017

@IanLee1521 - these are thoughtful questions, here are my (hopefully) thoughtful responses...

1.) Yes, for now, changes to the schema revolve around changing that HTML file. I would take your suggestion a bit further - the core file should be json (in the json-schema format). If the file is more machine-readable, we can apply automated tests and it makes it far less likely that human errors will be introduced. Any HTML, markdown, or sample json can also be generated based upon the schema file.

2.) The issue that v1.0.0 of the schema had with the organization field was resolved by moving organization into each of the objects in projects. I liked your suggestion to resolve the issue by allowing the code.json to have an array of json objects, but unfortunately, that's not where we shook out for v1.0.1 of the spec.

3.) This isn't a bug, but perhaps it could be called out / visualized differently. I believe the intent was to note that the repository field is not required under certain conditions (despite being listed in the required section of the schema definition).

I'm closing this issue since we've pushed our revisions to production (see https://code.gov/#/policy-guide/docs/compliance/inventory-code).

@IanLee1521
Copy link
Contributor

@michael-balint - Sounds reasonable. Perhaps we should open a new issue to track the idea of moving the core schema to a json-schema from the final HTML form?

@michael-balint
Copy link
Contributor

michael-balint commented Jan 11, 2017 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants