Skip to content
This repository has been archived by the owner on Mar 22, 2022. It is now read-only.

Additional linting + JSON Schema for config.json #152

Open
sshine opened this issue Dec 12, 2018 · 4 comments
Open

Additional linting + JSON Schema for config.json #152

sshine opened this issue Dec 12, 2018 · 4 comments

Comments

@sshine
Copy link

sshine commented Dec 12, 2018

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.)
    4. "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 Decide whether the exercises in config.json should follow any specific order haskell#790), or that "unlocked_by" must point to an actual exercise. (Again, I'm not sure exactly what linting rules currently apply.)

@kytrinyx
Copy link
Member

@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
Copy link
Member

kytrinyx commented Dec 26, 2018

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

  • Core exercises first, in the order in which they appear
  • Side exercises second, in alphabetical order
  • Deprecated exercises third, perhaps in alphabetical order, but this doesn't matter much

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

@cmccandless
Copy link

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 🤷‍♂️), but a proper schema validation will fix that as well.

@valentin-p
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants