Require that version_removed is either a string or true

[foolip]

Setting it explicitly to null or false is meaningless.

[foolip]

This still needs work to actually enforce it, but I'm not sure if it's should go in compat-data.schema.json, in test-versions.js, or both?

[ddbeck]

Change compat-data.schema.json. Then stuff like the VS Code settings will automatically pick this up. Let's mess with the linter only if there's confusion around the errors raised by the schema change.

[foolip]

Since it's not obvious: the "true", "false" and "null" here merely disallow those strings, presumably to help highlight such mishaps in the editor. (The linter would also complain about it.)

[ddbeck]

Yep. History of this is in #485.

[foolip]

This is not the most obvious way of saying that true is an allowed value, but convenient since the string "true" is already excluded here.

[foolip]

@ddbeck I've updated compat-data.schema.json now.

[ddbeck]

A couple little suggestions on the wording of the descriptions, but nothing major. Also, I checked out this branch locally and confirmed that the schema changed worked as expected by seeing my editor correctly flag some now-invalid changes. 👍

[ddbeck]

Yep. History of this is in #485.

[foolip]

@ddbeck suggestions applied!

[ddbeck]

Thank you!

[ddbeck]

@Elchi3 this is a (what looks to me a non-breaking) schema change, so I thought I'd give you a chance to review it too. Though if you're not concerned about it, dismiss the review request and I'll go ahead.

[foolip]

Given that the data and schema go together, what would constitute a breaking schema change?

Since this change makes the schema stricter it should be breaking by some definition, but I'm not sure how in practice.

[ddbeck]

This was my thinking: suppose we changed the data, but didn't change the schema. That wouldn't be breaking, though it would be the case that our data, at present, doesn't contain any "version_removed": false or "version_removed": null data (but still could, in the future—this is current state of non-real values in css for example). Consumers might (should, even) have code to handle such data, but none would appear in practice.

But by changing the schema, we're saying to consumers, "you can safely discard any code that expects "version_removed": false or "version_removed": null". Now that I've spelled this out, this sounds like a feature, so it's semver minor (e.g., the next version could be v3.2.0, not v3.1.4).

That said, if we wanted to play things safe, we could treat any schema change as breaking.

[foolip]

Ah, right, the schema is our "API" and consumers can now make some more assumptions about the data, which is exactly why I made the change.

I don't particularly mind which part of the version is bumped, but could see the argument for either minor or patch version. Bumping the minor version might bring more attention to the change, allowing more consumers to simplify their code.

[Elchi3]

Makes sense to me! We should have had this from this start :)
Semver minor sounds good to me, too.

[ddbeck]

Thanks, @foolip and @Elchi3! 🎉

I've started draft PR #9404 for this week's release.