travis: Choose schema based on USE_OLD_SCHEMA file (strict) by petertseng · Pull Request #1074 · exercism/problem-specifications

petertseng · 2018-01-05T04:52:23Z

The strict version of #1072: Enforces that we will not forget to remove the USE_OLD_SCHEMA file.

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
commit "travis: Choose schema based on USE_OLD_SCHEMA file" can/should
be reverted and then all exercises will in fact use the new schema,
which will replace the old completely.

petertseng · 2018-01-05T04:56:21Z

Warning. Confusing CI output.

exercises/alphametics/canonical-data.json valid
exercises/alphametics/canonical-data.json invalid
[ { keyword: 'oneOf',
    dataPath: '.cases[0]',
    schemaPath: '#/oneOf',
    params: {},
    message: 'should match exactly one schema in oneOf' } ]

This is saying:

alphametics is valid under the old schema.
alphametics is invalid under the new schema.

Since alphametics intends to use the old schema, it must be the case that it is valid under the old schema and invalid under the new schema, so this is correct, but confusing.

I'd suggest that I suppress the output from the invalid-under-new check!

petertseng · 2018-01-05T05:13:26Z

#1075 demonstrates all three possible modes of failure.

petertseng · 2018-01-05T18:44:52Z

Rebased

petertseng · 2018-01-05T18:51:23Z

                { "description": { "$ref": "#/definitions/description" }
                , "comments"   : { "$ref": "#/definitions/comments"    }
                , "property"   : { "$ref": "#/definitions/property"    }
+                , "input"      : { "$ref": "#/definitions/input"    }


how did I cause this to get misaligned?

Because I was lazy in #998.

petertseng · 2018-01-16T01:05:17Z

Excuse me for a moment. Temporarily need to commandeer this PR to test #1129 and #1130. I refuse to merge or review input object PRs without having them be tested.

ErikSchierboom · 2018-02-09T14:58:22Z

@petertseng I was wondering if you could re-run the script to see how many (and which) exercises don't conform to the new schema?

petertseng · 2018-02-09T15:09:22Z

The way to track progress is to merge this PR, not to keep rebasing it. However, you are in luck, because to merge this it needs to be rebased anyway.

rpottsoh · 2018-02-09T18:58:59Z

Ugh, request for review request was accidental on my part...

ErikSchierboom · 2018-02-10T11:42:01Z

@petertseng This PR looks good and the CI is green. Is there anything we should be waiting for before we merge this?

Derived from the list of invalid exercises listed in: #998 (comment)

This schema was proposed and accepted in #996

petertseng · 2018-02-10T11:46:20Z

Is there anything we should be waiting for before we merge this?

No, there is nothing to wait for.

As a rule, if I have not marked a PR as "DO NOT MERGE" I intend for it to be merged. But nobody Approved (in the GitHub sense of the word), so I did not merge it.

Only potential thing to watch out for:

If some exercise has moved to the new schema since the last time this PR was rebased[1], then the CI will appear to pass, but will actually fail when this PR is merged.

[1]: Not quite true, since Travis CI checks the result of merging, but I'd need to figure out whether Restart Build button recalculates the commit to merge with.

petertseng · 2018-02-10T11:57:35Z

+        if ajv -s canonical-schema.json -d $json 2>/dev/null; then
+          forgotten="$forgotten $ex"
+        fi
+        schema="old-schema.json"


sorry, what? is this necessary?

Yeah no, it was not necessary. This line has been removed.

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

OK, With apologies, I made some changes since the last approval, but I have confidence in them, and if I'm wrong, the responsibility on that lies with me alone.

git diff -u 3051deb36d2293baff9f3e5a47fa9e391f7ac388 HEAD

diff --git a/.travis.yml b/.travis.yml
index 7b98a41..b129a48 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -15,7 +15,6 @@ script:
       dir=$(dirname $json)
       ex=$(basename $dir)
 
-      schema="canonical-schema.json"
       if [ -f "$dir/USE_OLD_SCHEMA" ]; then
         if ! ajv -s old-schema.json -d $json; then
           invalid_old="$invalid_old $ex"
@@ -25,13 +24,11 @@ script:
         if ajv -s canonical-schema.json -d $json 2>/dev/null; then
           forgotten="$forgotten $ex"
         fi
-        schema="old-schema.json"
       else
         if ! ajv -s canonical-schema.json -d $json; then
           invalid_new="$invalid_new $ex"
         fi
       fi
-
     done
 
     stat=0

I am pretty sure those schema lines were in there because of #1072, and were not being used in this PR.

petertseng · 2018-02-10T12:04:15Z

+          forgotten="$forgotten $ex"
+        fi
+      else
+        if ! ajv -s canonical-schema.json -d $json; then


this could have been an elsif but I didn't think it would make the code sufficiently clear.

This reverts commit a866c43. Observe that there are no more USE_OLD_SCHEMA files in this repository. This indicates that henceforth all canonical-data.json files should use the schema proposed in #996 and defined in #1074.

petertseng mentioned this pull request Jan 5, 2018

travis: Choose schema based on USE_OLD_SCHEMA file #1072

Closed

petertseng mentioned this pull request Jan 5, 2018

DO NOT MERGE: we broke it in all three ways #1075

Closed

petertseng commented Jan 5, 2018

View reviewed changes

petertseng mentioned this pull request Jan 15, 2018

DO NOT MERGE: trial new schema w/ input #998

Closed

rpottsoh requested a review from Stargator February 9, 2018 18:57

rpottsoh removed the request for review from Stargator February 9, 2018 19:01

petertseng added 2 commits February 10, 2018 03:45

mark exercises using old schema

2f07018

Derived from the list of invalid exercises listed in: #998 (comment)

travis: Evaluate ALL exercises with new schema

b3d6488

This schema was proposed and accepted in #996

ErikSchierboom approved these changes Feb 10, 2018

View reviewed changes

petertseng commented Feb 10, 2018

View reviewed changes

petertseng merged commit c7aa0ff into exercism:master Feb 10, 2018

petertseng deleted the ci-new-schema-strict branch February 10, 2018 12:09

This was referenced Feb 10, 2018

JSON Schema proposal: All inputs should appear under the input key #996

Closed

Revert "travis: Choose schema based on USE_OLD_SCHEMA file" #1193

Merged

Uh oh!

Conversation

petertseng commented Jan 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petertseng commented Jan 5, 2018

Uh oh!

petertseng commented Jan 5, 2018

Uh oh!

petertseng commented Jan 5, 2018

Uh oh!

petertseng Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

petertseng Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

petertseng commented Jan 16, 2018

Uh oh!

ErikSchierboom commented Feb 9, 2018

Uh oh!

petertseng commented Feb 9, 2018

Uh oh!

rpottsoh commented Feb 9, 2018

Uh oh!

ErikSchierboom commented Feb 10, 2018

Uh oh!

petertseng commented Feb 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petertseng Feb 10, 2018

Choose a reason for hiding this comment

Uh oh!

petertseng Feb 10, 2018

Choose a reason for hiding this comment

Uh oh!

petertseng left a comment

Choose a reason for hiding this comment

Uh oh!

petertseng Feb 10, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

petertseng commented Jan 5, 2018 •

edited

Loading

petertseng commented Feb 10, 2018 •

edited

Loading