-
-
Notifications
You must be signed in to change notification settings - Fork 616
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
Add prerequisites for practice exercises #1468
Add prerequisites for practice exercises #1468
Conversation
Dear TomPradatThank you for contributing to the JavaScript track on Exercism! 💙
Dear Reviewer/Maintainer
Automated comment created by PR Commenter 🤖. |
config.json
Outdated
"prerequisites": [ | ||
"classes", | ||
"array-transformation", | ||
"objects", | ||
"strings", | ||
"template-strings" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like "strings" is a duplicate of "template-strings", but I'm not 100% sure about adding "template-strings" here, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This exercise is a tough one to define. The proof file solution is not very good imo and the community solutions are full of awful solutions as well.
I digged around a bit and I would use something like this as guidance for the prerequisites: https://exercism.org/tracks/javascript/exercises/nucleotide-count/solutions/shybyte
but with two changes
- instead of the spread operator (
...
) there could be a simplesplit
- you need a template string instead of Object.values(...).join in the end because the order is not guaranteed otherwise
That would leave us with classes, objects, template-strings, array-loops, conditionals and errors as prerequisites. What do you think about that?
I think we don't need to list strings as well because template strings will always come after "strings" in the concept tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I've done something similar myself: https://exercism.org/tracks/javascript/exercises/nucleotide-count/solutions/TomPradat
I'm wondering about conditionals though, it seems to me that 4 prerequisites are long enough. What's more, conditionals should be known before learning about objects. What do you think?
May I have a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left my thoughts on the exercises in the PR. The issue description also mentions "meetup". Not sure what's up with that. 🤔
config.json
Outdated
"prerequisites": [ | ||
"classes", | ||
"array-transformation", | ||
"objects", | ||
"strings", | ||
"template-strings" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This exercise is a tough one to define. The proof file solution is not very good imo and the community solutions are full of awful solutions as well.
I digged around a bit and I would use something like this as guidance for the prerequisites: https://exercism.org/tracks/javascript/exercises/nucleotide-count/solutions/shybyte
but with two changes
- instead of the spread operator (
...
) there could be a simplesplit
- you need a template string instead of Object.values(...).join in the end because the order is not guaranteed otherwise
That would leave us with classes, objects, template-strings, array-loops, conditionals and errors as prerequisites. What do you think about that?
I think we don't need to list strings as well because template strings will always come after "strings" in the concept tree.
config.json
Outdated
@@ -629,7 +629,13 @@ | |||
"name": "Nucleotide Count", | |||
"uuid": "be6f3ab3-1593-4c2d-8a35-5f39c1d5c91f", | |||
"practices": [], | |||
"prerequisites": [], | |||
"prerequisites": [ | |||
"classes", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: "classes" can be removed here, the exercise was changed to remove the unnecessary class definition (#1488)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to create an issue about this, good to know it has been fixed :)
Oops, looks like I forgot to push this one's commit. It's done now :) |
This PR is related to #983 , it will deal with the following exercises :
In order to evaluate the prerequisites in the best way, I'm doing the exercise then I'm reviewing the exercise's proof of work and the submitted solutions.