exercism / v2-configlet

Tool to assist in managing Exercism language tracks.
MIT License
16 stars 23 forks source link

Additional linting + JSON Schema for config.json #152

Open sshine opened 5 years ago

sshine commented 5 years ago

In exercism/haskell#790 we discuss the order of exercises in config.json.

To automatically re-arrange exercises in config.json I started building a tool, not realizing that #117 existed and requests for this to be added as part of configlet; a more noble effort.

While building this tool I wrote a typed model of config.json that led me to the following corners:

1) "topics" can either be a list of strings or null, which presumably means the same as []. Does null make sense over []? 2) "auto_approve" is optional and defaults to false. Does it make sense to allow explicit false? 3) "deprecated" is optional and defaults to false. Does it make sense to allow explicit false? 4) "unlocked_by" is mandatory, but can be null or a string.

The combination of "core": true/false and "unlocked_by": "something"/null creates four combinations: 1) "core": true + "unlocked_by": null appears to mean "core exercise". 2) "core": false + "unlocked_by": "something" appears to mean "side exercise". 3) "core": false + "unlocked_by": null appears to mean "always available side exercise"? (Unless "deprecated": true.) 2) "core": true + "unlocked_by": "something" is not used, since core exercises unlock by order.

I propose that we:

  1. Revise the specification (linked to from configlet's README.md): I think "problems" is deprecated and it's now called "exercises"?

  2. Address the ambiguities highlighted above; ideally, resolve them in a way that provides a canonical form (e.g. "Don't allow "auto_approve": false, leave it out." and ""topics" is always a list.") and clarify the meaning of "core" + "unlocked_by" or disallow certain combinations. Express the formatting rules as a JSON Schema for config.json so that we can programmatically validate the specification with configlet.

  3. Expand on configlet's high-level constraints (ones not easily encoded as JSON Schema) that we assume, e.g. that side exercises can't unlock other exercises (I don't know if this is true, or if the opposite is intended, but it's not clear that it is or should eventually be possible), or that exercises must be ordered a certain way in the file (as discussed in exercism/haskell#790), or that "unlocked_by" must point to an actual exercise. (Again, I'm not sure exactly what linting rules currently apply.)

kytrinyx commented 5 years ago

@sshine Thank you so much for tackling this question and helping tease out all of these ambiguities. This is a massive help.

"topics" can either be a list of strings or null, which presumably means the same as []. Does null make sense over []?

They indicate different things. null means that the maintainers have not made a determination. [] means that the maintainers agree that there are no relevant topics worth mentioning.

"auto_approve" is optional and defaults to false. Does it make sense to allow explicit false? "deprecated" is optional and defaults to false. Does it make sense to allow explicit false?

I'd actually prefer not to have the key present if false, since it would potentially add a huge amount of noise (very few exercises will be auto-approvable, typically only one per track, and only a handful will be deprecated).

I think "problems" is deprecated and it's now called "exercises"?

Yes. I think I managed to get rid of any reference to problems, too, which would mean that we can, indeed, get rid of it in the README. Good call!

[I propose that we] clarify the meaning of "core" + "unlocked_by" or disallow certain combinations.

Agreed.

[I propose that we E]xpress the formatting rules as a JSON Schema for config.json

Yes, 100% yes. JSON schema is the obvious choice here.

that side exercises can't unlock other exercises

At the moment this is true, though we've been back and forth on it. For now we've concluded that the additional complexity is unnecessary.

kytrinyx commented 5 years ago

Per https://github.com/exercism/haskell/issues/790 the additional linting described in this issue would also include ensuring that the config.json exercises are ordered per the following rules:

This behavior should be behind a flag so that we can release the new tool without all the tracks failing.

cmccandless commented 5 years ago

Came to this repo to report a bug that configlet lint does not fail on duplicate properties (didn't know this was even valid in JSON :man_shrugging:), but a proper schema validation will fix that as well.

valentin-p commented 4 years ago

Per exercism/v3#2272 and exercism/v3#2279. We "could" order the exercise concept list by slug alphabetically