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

Add prerequisites #2159

Merged
merged 5 commits into from
Jun 12, 2023
Merged

Conversation

safwansamsudeen
Copy link
Contributor

As part of #984, I've added the prerequisites for the House exercises and removed the topics key as per Erik's comment in #960. Note that many entries with prerequisites still have topics - this should be addressed, maybe?

I just completed this exercise myself. This is the top solution and it uses arrays, something needed to implement this exercise without hardcoding values. You also need to learn strings, along with string manipulation (I'm not sure if we should add to the practices key in this PR: when I get the clarification, I'll add template-strings to it). Finally, all the top solutions use for loops - perhaps this could be done using map or while, but the most straightforward solution will use for loops.

I apologize for the sync exercise commit and it's reversion - it happened due to a conflict with another issue I opened. New-ish to git!

@github-actions
Copy link
Contributor

Hello. Thanks for opening a PR on Exercism. We are currently in a phase of our journey where we have paused community contributions to allow us to take a breather and redesign our community model. You can learn more in this blog post. As such, all issues and PRs in this repository are being automatically closed.

That doesn't mean we're not interested in your ideas, or that if you're stuck on something we don't want to help. The best place to discuss things is with our community on the Exercism Community Forum. You can use this link to copy this into a new topic there.


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.

@safwansamsudeen
Copy link
Contributor Author

@SleeplessByte thanks for reopening this. I realize that you're currently not accepting CCs, but @TomPradat specifically said he'll look into this when he has time.

Copy link
Contributor

@TomPradat TomPradat left a comment

Choose a reason for hiding this comment

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

I myself have done this exercise using recursion, but given the difficulty of the exercise and given the place of the "recursion" concept in the concepts tree, I believe it's a good idea not to put it in the prerequisites 👍

IMO there is no need to add template strings here. And I totally agree with the other prerequisites you have added. For loop is the most straightforward I believe too.

No worries about the sync exercise commit, these commits will all be squashed anyways.

Thank you for working on this !

PS: Sorry for the time it took me to review this PR, I had a lot to deal with

@safwansamsudeen
Copy link
Contributor Author

safwansamsudeen commented Jun 12, 2023

@TomPradat thanks for the review. Let me know when you're free again so that I can submit PRs for the rest of the exercises. What about merging this?

Yeah, we don't need template-strings in prerequisites - I meant that we add it to practices, as Erik has said.

@SleeplessByte SleeplessByte added the x:rep/small Small amount of reputation label Jun 12, 2023
@SleeplessByte SleeplessByte merged commit aa60afb into exercism:main Jun 12, 2023
@TomPradat
Copy link
Contributor

@TomPradat thanks for the review. Let me know when you're free again so that I can submit PRs for the rest of the exercises. What about merging this?

You can submit PRs one by one. Don't expect me to be quick these days at reviewing but I'll do it gladly when I have some time 🙂.

Yeah, we don't need template-strings in prerequisites - I meant that we add it to practices, as Erik has said.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:rep/small Small amount of reputation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants