Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Starting on concepts. #128

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

redvers
Copy link

@redvers redvers commented Aug 27, 2022

Initial commit will fail CI as the exercise isn't there yet.

@redvers redvers force-pushed the first_concept_variables branch from 221da9b to 25e47b7 Compare August 27, 2022 03:18
Copy link
Member

@ee7 ee7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Came here from Slack).

The configlet lint output of:

The Concept Exercise lucians-luscious-lasagna has basics-1 in its concepts, which is not a slug in the top-level concepts array:
/home/runner/work/pony/pony/config.json

is trying to point out that the exercise claims to teach the basics-1 concept, but that concept does not exist here:

"concepts": [],

To resolve that output, we need to add something like:

  "concepts": [
    {
      "uuid": "753569f6-14f1-4e9d-ab70-db4819a17162",
      "slug": "basics-1",
      "name": "Basics 1"
    }

(Although I think we generally just call it basics, and then have stuff that doesn't fit in "basics" as separate concepts).

Then re-running configlet lint will produce some new errors complaining that the concept's files don't exist.

See e.g.

@redvers
Copy link
Author

redvers commented Aug 27, 2022

Ah ha! I apparently had concept at the top level twice. I had an entry that matches (I think) what you describe at line 11, and then undefined it at line 186.

So I just pushed a change which removed line 186 and… it still fails with the same response.

This is what it looks like:

https://github.com/redvers/pony/blob/5cbf3657a8210540772b396d6102aecfafec5132/config.json#L11-L17

I don't see how it looks different to your example.

(edit: I re-ran the linter in debug and the error changed state to the state you said would be next up - thank you)

@ee7
Copy link
Member

ee7 commented Aug 27, 2022

Ah. I didn't catch that concepts was defined twice. Sorry.

A duplicate key name is valid JSON, but generally recommended against. For one thing, it's less portable, as different parsers handle it differently. But it's common to silently use the last value, and that's what the Ruby library that powers the Exercism website does.

configlet lint also currently silently uses the last value. I have proposed that it should indicate an error when there is a duplicate key. I'm tracking this in exercism/configlet#347.

But it turns out that we don't hit this problem much in practice, so this hasn't been a priority.


In the future, configlet lint should probably issue warnings rather than errors for missing files when the track isn't launched, or when concept exercises aren't enabled. The status quo encourages adding empty files to appease the linter, but this makes it harder to keep track of files that still need to be written.

"concepts": [
{
"uuid": "3e6e4a21-cc66-4c48-857b-90e1ca5298f9",
"slug": "basics-1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rename the concept to basics? There should really only be one basics concept. Any other concepts should be "real" concepts (like numbers, booleans, etc.)

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

Successfully merging this pull request may close these issues.

3 participants