Skip to content

travis: Choose schema based on USE_OLD_SCHEMA file#1072

Closed
petertseng wants to merge 3 commits intoexercism:masterfrom
petertseng:ci-new-schema
Closed

travis: Choose schema based on USE_OLD_SCHEMA file#1072
petertseng wants to merge 3 commits intoexercism:masterfrom
petertseng:ci-new-schema

Conversation

@petertseng
Copy link
Copy Markdown
Member

If it is present, use the old schema, else use the new schema.

This plan is proposed in:
#998

Its merits are that it allows us to:

  • Track progress by simply counting number of USE_OLD_SCHEMA file.
  • Verify, as each PR is made, that each JSON file intending to use new
    schema does in fact validate.
    • Otherwise we have to manually run a schema check at periodic
      intervals.
  • Keep compatibility with old schema, to support moving over one file at
    a time instead of all at once.

Note that old-schema.json is the same canonical-schema.json as is
currently on master.

The intention is that when all exercises use the new schema, the last
commit in this PR can/should be reverted
and then all exercises will in fact use
the new schema, which will replace the old completely.

@petertseng
Copy link
Copy Markdown
Member Author

petertseng commented Jan 5, 2018

As we can see in #1073, we enforce the condition: "If USE_OLD_SCHEMA is not present, the file is a correct file under the new schema".

Note that since the new schema is a superset of the old, I have a difficult time enforcing the converse condition: "If the file is a correct file under the new schema, USE_OLD_SCHEMA is not present".

This means someone may convert a JSON file to the new schema and then forget to remove USE_OLD_SCHEMA, and we have no way of detecting it.

If it is very important, we can take the strategy: "If USE_OLD_SCHEMA is present, also enforce that the file is invalid under the new schema". That will enforce that we do not forget to remove the USE_OLD_SCHEMA file. The cost is simply extra complexity in .travis.yml

This strategy is implemented in #1074

@petertseng
Copy link
Copy Markdown
Member Author

Uh guess it behooves me to rebase this now that alphametics is on the thing

Derived from the list of invalid exercises listed in:
#998 (comment)
Comment thread canonical-schema.json Outdated
{ "description": { "$ref": "#/definitions/description" }
, "comments" : { "$ref": "#/definitions/comments" }
, "property" : { "$ref": "#/definitions/property" }
, "input" : { "$ref": "#/definitions/input" }
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

how did I cause this to get misaligned?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because I was lazy in #998.

This schema was proposed and accepted in
#996
If it is present, use the old schema, else use the new schema.

This plan is proposed in:
#998

Its merits are that it allows us to:

* Track progress by simply counting number of USE_OLD_SCHEMA file.
* Verify, as each PR is made, that each JSON file intending to use new
  schema does in fact validate.
  * Otherwise we have to manually run a schema check at periodic
    intervals.
* Keep compatibility with old schema, to support moving over one file at
  a time instead of all at once.

Note that old-schema.json is the same canonical-schema.json as is
currently on master.

The intention is that when all exercises use the new schema, **this
commit can/should be reverted** and then all exercises will in fact use
the new schema, which will replace the old completely.
@petertseng
Copy link
Copy Markdown
Member Author

Because #1074 exists, I refuse to maintain this PR any further. I do not prefer the ability to forget to remove a USE_OLD_SCHEMA file.

@petertseng petertseng closed this Jan 15, 2018
@petertseng petertseng deleted the ci-new-schema branch January 15, 2018 22:20
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.

1 participant