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

WIP: starting to restructure fixtures and modify track config struct #187

Draft
wants to merge 1 commit into
base: v3-upgrades
Choose a base branch
from

Conversation

ekingery
Copy link
Contributor

@ekingery ekingery commented Oct 23, 2020

@ErikSchierboom I wanted to get this PR going early so that you could validate the V3 changes to the fixtures/lint/valid-track/config.json before I spend too much time writing code against them.

@ekingery
Copy link
Contributor Author

The code changes for linting should be fairly straightforward, but I'm concerned about the scope of the repercussions to the rest of the features (fmt, tree, generate). This is not yet an informed opinion, but my gut says that it might be faster and easier to essentially rewrite these tools using json-schema (or maybe joi) as suggested in #152. For your consideration, @ErikSchierboom.

Comment on lines +44 to +45
"uuid": "2CE9984C-64C1-41F3-AEBA-037F8C06616F",
"slug": "aluminum",
Copy link
Member

Choose a reason for hiding this comment

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

Just for consistency with the concept exercises:

Suggested change
"uuid": "2CE9984C-64C1-41F3-AEBA-037F8C06616F",
"slug": "aluminum",
"slug": "aluminum",
"uuid": "2CE9984C-64C1-41F3-AEBA-037F8C06616F",

Comment on lines +46 to +47
"topics": [],
"teaches": [],
Copy link
Member

Choose a reason for hiding this comment

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

Note that the topics property is only temporary. It will be used to help with filling the prerequisites key:

Suggested change
"topics": [],
"teaches": [],
"topics": ["conditionals", "optional_values", "text_formatting"],

"topics": [],
"teaches": [],
"prerequisites": ["basics", "strings"],
"difficulty": 1
Copy link
Member

Choose a reason for hiding this comment

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

Also note that practice exercises can have an optional deprecated property:

"deprecated": true

type PracticeMetadata struct {
Slug string `json:"slug"`
UUID string `json:"uuid"`
IsCore bool `json:"core"`
Copy link
Member

Choose a reason for hiding this comment

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

This property will be removed in v3

Slug string `json:"slug"`
UUID string `json:"uuid"`
IsCore bool `json:"core"`
AutoApprove bool `json:"auto_approve,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

This property will be removed in v3

UUID string `json:"uuid"`
IsCore bool `json:"core"`
AutoApprove bool `json:"auto_approve,omitempty"`
UnlockedBy *string `json:"unlocked_by"`
Copy link
Member

Choose a reason for hiding this comment

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

This property will be removed in v3

AutoApprove bool `json:"auto_approve,omitempty"`
UnlockedBy *string `json:"unlocked_by"`
Difficulty int `json:"difficulty"`
Topics []string `json:"topics,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

The prerequisites field also needs to be added:

Suggested change
Topics []string `json:"topics,omitempty"`
Prerequisites []string `json:"prerequisites,omitempty"`
Topics []string `json:"topics,omitempty"`


// ConceptMetadata contains metadata about an implemented concept exercise.
// The order below determines the order delivered by the API.
type ConceptMetadata struct {
Slug string `json:"slug"`
UUID string `json:"uuid"`
IsCore bool `json:"core"`
Copy link
Member

Choose a reason for hiding this comment

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

This property does not exist for concept exercises.


// ConceptMetadata contains metadata about an implemented concept exercise.
// The order below determines the order delivered by the API.
type ConceptMetadata struct {
Slug string `json:"slug"`
UUID string `json:"uuid"`
IsCore bool `json:"core"`
AutoApprove bool `json:"auto_approve,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

This property does not exist for concept exercises.


// ConceptMetadata contains metadata about an implemented concept exercise.
// The order below determines the order delivered by the API.
type ConceptMetadata struct {
Slug string `json:"slug"`
UUID string `json:"uuid"`
IsCore bool `json:"core"`
AutoApprove bool `json:"auto_approve,omitempty"`
UnlockedBy *string `json:"unlocked_by"`
Copy link
Member

Choose a reason for hiding this comment

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

This property does not exist for concept exercises.

Slug string `json:"slug"`
UUID string `json:"uuid"`
IsCore bool `json:"core"`
AutoApprove bool `json:"auto_approve,omitempty"`
UnlockedBy *string `json:"unlocked_by"`
Difficulty int `json:"difficulty"`
Topics []string `json:"topics"`
Topics []string `json:"topics,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

The topics key does not exist. It should be replaced with:

Suggested change
Topics []string `json:"topics,omitempty"`
Concepts []string `json:"concepts,omitempty"`
Prerequisites []string `json:"prerequisites,omitempty"`

@ErikSchierboom
Copy link
Member

The code changes for linting should be fairly straightforward, but I'm concerned about the scope of the repercussions to the rest of the features (fmt, tree, generate). This is not yet an informed opinion, but my gut says that it might be faster and easier to essentially rewrite these tools using json-schema (or maybe joi) as suggested in #152. For your consideration, @ErikSchierboom.

I like the idea of using JSON schema to validate the track config.

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

Successfully merging this pull request may close these issues.

2 participants