-
Notifications
You must be signed in to change notification settings - Fork 519
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 Exercise Matrix #1877
Add Exercise Matrix #1877
Conversation
Hello. Thanks for opening a PR on Exercism 🙂 We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in. You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch. If you're interested in learning more about this auto-responder, please read this blog post. Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it. |
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.
Haven't done a detailed review yet, but it looks like you're not using the Tera template to generate the tests. Please do that so we can keep the test suite up-to-date easily. Fastest way to regenerate tests from the template: just update-exercise --slug word-count
(need just
installed)
You can find some tips about writing the Tera template in CONTRIBUTING.md
.
I'd say difficulty 4 is fine. Looking through the config file, there are many other exercises at difficulty 4 which I would categorize as "light data structure handling". |
Sorry about that. I realized when I forked the repo the I think I got the hang of it and everything should be correct now. |
1 similar comment
Sorry about that. I realized when I forked the repo the I think I got the hang of it and everything should be correct now. |
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.
Great work! Just one more request, then I'll merge.
I think it's important to stick to problem-specifications as closely as reasonable and document justified deviations verbosely.
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.
Thank you for the great work!
Per forum post - Adding Matrix to the Rust Track
Here's what I did:
rust-tooling/generate
This is my first contribution so there's a chance I probably forgot something but it looks like all cargo and CI tests are running on my end, so hopefully we should be good.
Also, I increased the difficulty to 4 as depending on the implementation you could be working with nested iterators/map/collect, but I was going back and forth on this. If you think 1 is still more appropriate, just let me know and I'll switch it back.
Thanks!