Addition of a new schema #170

teoli2003 · 2017-05-04T10:51:34Z

This is the first PR resulting in the division of PR #86.

Renaming of the old schema
Adding the new schema
Adding the docs for the new schema
Dividing the travis tests so that it will be easier to merge future PR for the migration of the existing data.

…est on several entries to make migration easier

Elchi3

I think I would like to land this initial schema as is now. I opened #173 to change additionalProperties to false for compat_block and there are a few other open issues, but I don't think these block the initial landing. Let's get familiar with the new schema some more by landing it and iterating on the smaller issues we identified.

Elchi3 · 2017-05-04T13:37:20Z

Will, this makes travis also test the new WebExtension JSON that landed yesterday with the new schema. What do you think about merging this?

wbamberg

There are quite a few comments on the doc, some of which seem like errors, and some of which are just extra clarifications.

I'm wondering even whether we should have a doc at this level of detail: it's easy for it to get out of step with the schema, and the schema is really the source of truth. We could have a much shorter overview doc, and leave the details to the schema.

But I do think maybe we should have a "Checklist for reviewers" doc, that covers anything not enforced by the schema.

There are a couple of comments on the schema itself, but in general it's looking good! I'd also like to register my discomfort at including HTML and KS macro calls in notes.

wbamberg · 2017-05-04T16:05:05Z

compat-data-schema.md

@@ -0,0 +1,365 @@
+# The browser-compat-data JSON format


I think this doc could do with a copy-edit, but that can be done in a follow-up.

Also the README should be updated to point to this document: at the moment it documents the old schema.

I created issue #177 for the copy-edit part.
I updated the README file to link to the compat-data-schema.md instead of the description of the old schema.

wbamberg · 2017-05-04T16:08:02Z

compat-data-schema.md

+browser-compat-data JSON format. Nevertheless there are constraints that cannot
+be described inside a schema, and won't be checked by automated tests. We tried
+to mark such cases in the following text with a __Not enforced by the
+schema__ notice.


This notice is not actually used anywhere.

I removed the whole note. As @wbamberg said it is not used, and as we will create a specific doc for reviewer (see issue #176), it will never be.

wbamberg · 2017-05-04T16:08:45Z

compat-data-schema.md

+by making it easy to maintain by humans.
+
+### Schema versioning
+There one single information that is tied to each file, and is mandatory. It is


There are actually 2 pieces of mandatory data at the top level (version and data).

wbamberg · 2017-05-04T16:10:44Z

compat-data-schema.md

+ The hierarchy of identifiers is not defined inside the schema. It is a
+ convention of the project using the schema.
+
+ In the MDN browser-compat case, it is:


I'm not sure we should say this. It's already out of date :). We could just say "for example" instead, or omit it.

To avoid any confusion, I removed it (there is an theoretical example a few lines before)

wbamberg · 2017-05-04T16:12:12Z

compat-data-schema.md

+explained in the previous paragraph, any feature has at least one sub-feature
+called `'basic_support'`, but it may many more.
+
+A sub-feature may have three properties.


Let's be clear which are optional and which are mandatory (as we just discussed, perhaps "support" is mandatory and the other two are optional).

wbamberg · 2017-05-04T16:19:47Z

compat-data-schema.md

+Compatibility information is stored in a `"support"` field. It may consist of the
+following properties:
+
+#### `"version_added"`


Could note that this is the only mandatory property.

wbamberg · 2017-05-04T16:20:11Z

compat-data-schema.md

+
+Each `string` that contains a human-readable description of the sub-feature. The
+`<code>` and `<a>`, as well as the macros `{{cssxref}}`, `{{HTMLElement}}`,
+`{{htmlattrxref}}`, and `{{domxref}}` can be used. See the localization section


See comments above about this.

Yep, same: KS macros should have been removed. This part is fixed.

wbamberg · 2017-05-04T16:23:33Z

compat-data-schema.md

+`text-align` property is identified by `css.properties.text-align`.
+
+The strings of an identifier are not intended to be displayed and are therefore
+not translatable.


Could note somewhere that identitifers can't contain dots.

I think that technically they can (not saying it is a good idea to do it)

wbamberg · 2017-05-04T16:24:43Z

compat-data.schema.json

-        "Safari Mobile": { "$ref": "#/definitions/support_statement" },
-        "Servo": { "$ref": "#/definitions/support_statement" },
+        "desc": { "type": ["string"] },
+        "support": { "$ref": "#/definitions/compat_block" },


As discussed, we could make "support" mandatory.

wbamberg · 2017-05-04T16:27:30Z

compat-data.schema.json

      "additionalProperties": false
    },

-    "feature_set": {
+    "compat_block": {


Very much a nit, but it helps readability if you swap this and "subfeature", so I can read it all the way from the bottom up, going from a reference to its definition.

teoli2003 · 2017-05-05T06:07:28Z

So I made the requested changes except:

global copy-edit has not been done but is issue Copy-edit schema documentation #177 as suggested.
I didn't documented that '.' are not allowed in identifier, because I think they are (though they are not a good idea). If I'm wrong, this can be done in a follow-up too as the schema is not touched by it, only the docs.

(Note that one comment seems to be not fixed, because I fixed it by updating the text below)

@wbamberg @Elchi3 I think this is ready for a new round of review.

wbamberg · 2017-05-05T16:45:23Z

Thanks for all those fixes @teoli2003 , it looks great.

global copy-edit has not been done but is issue #177 as suggested.

Sounds good.

I didn't documented that '.' are not allowed in identifier, because I think
they are (though they are not a good idea). If I'm wrong, this can be done
in a follow-up too as the schema is not touched by it, only the docs.

You are right. We talked about this a couple of days ago and I see @Elchi3 filed an issue for it: #167. I saw some regexps in the schema and thought this had made its way into the schema already, but I was mistaken :).

teoli2003 force-pushed the new-schema branch 2 times, most recently from fe9f18c to f2b8d97 Compare May 4, 2017 10:58

teoli2003 added 4 commits May 4, 2017 12:36

Rename old schema

ea103d1

Add new schema and its documentation

b6f79ba

Update travis to use the new name of the old schema, and divide the t…

54398be

…est on several entries to make migration easier

Add travis test for new schema and webextensions

b18b1a2

teoli2003 force-pushed the new-schema branch from f2b8d97 to b18b1a2 Compare May 4, 2017 11:36

teoli2003 mentioned this pull request May 4, 2017

Update http browser compat to new schema #172

Closed

Elchi3 self-requested a review May 4, 2017 13:32

Elchi3 approved these changes May 4, 2017

View reviewed changes

Elchi3 requested a review from wbamberg May 4, 2017 13:35

Elchi3 added this to In Progress in Migration of compat data from MDN pages May 4, 2017

Elchi3 added the schema ⚙️ Isses or pull requests regarding the JSON schema files used in this project. label May 4, 2017

This was referenced May 4, 2017

Update Javascript browser compat to new schema #174

Closed

Update to new schema #86

Closed

wbamberg requested changes May 4, 2017

View reviewed changes

teoli2003 added 6 commits May 4, 2017 18:33

Update README to point to the new schema and its documentation

14587f6

Remove notice not used anywhere from the schema description

84625f2

Don't allow KS in strings

5e64f7b

Additional fixes after review for docs of schema

e2ceb58

Reorder entries in schema and make support mandatory

e306191

Last bits of feedback incorporated in docs

cc6f9ff

wbamberg approved these changes May 5, 2017

View reviewed changes

wbamberg merged commit 4cb0f23 into mdn:master May 5, 2017

Elchi3 moved this from In Progress / Review Needed to Done in Migration of compat data from MDN pages May 8, 2017

Elchi3 assigned teoli2003 May 8, 2017

Elchi3 removed this from Done in Migration of compat data from MDN pages May 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addition of a new schema #170

Addition of a new schema #170

teoli2003 commented May 4, 2017

Elchi3 left a comment

Elchi3 commented May 4, 2017

wbamberg left a comment •

edited

wbamberg May 4, 2017

wbamberg May 4, 2017

teoli2003 May 4, 2017

wbamberg May 4, 2017

teoli2003 May 4, 2017

wbamberg May 4, 2017

teoli2003 May 5, 2017

wbamberg May 4, 2017

teoli2003 May 5, 2017

wbamberg May 4, 2017

teoli2003 May 5, 2017

wbamberg May 4, 2017

teoli2003 May 4, 2017

wbamberg May 4, 2017

teoli2003 May 4, 2017

wbamberg May 4, 2017

teoli2003 May 4, 2017

wbamberg May 4, 2017

teoli2003 May 5, 2017

wbamberg May 4, 2017

teoli2003 May 5, 2017

teoli2003 commented May 5, 2017

wbamberg commented May 5, 2017 •

edited

Addition of a new schema #170

Addition of a new schema #170

Conversation

teoli2003 commented May 4, 2017

Elchi3 left a comment

Choose a reason for hiding this comment

Elchi3 commented May 4, 2017

wbamberg left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teoli2003 commented May 5, 2017

wbamberg commented May 5, 2017 • edited

wbamberg left a comment •

edited

wbamberg commented May 5, 2017 •

edited