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

[Space Age]: Update introduction and instructions #3483

Merged
merged 6 commits into from
Aug 8, 2023

Conversation

AndrewLawendy
Copy link
Contributor

@AndrewLawendy AndrewLawendy commented Jul 31, 2023

I submitted this PR because I found it challenging to understand the task solely through the provided instructions. In my experience, relying solely on test cases to understand a task may not be the most effective approach for students. Therefore, I hope that my contributions can help clarify the task for future students.

@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.

@github-actions github-actions bot closed this Jul 31, 2023
@AndrewLawendy
Copy link
Contributor Author

Hi @BethanyG, I'm confused as to how my previous PR was not automatically closed by the bot. I would appreciate it if you could take a look and let me know if there are any changes I need to make. If you find the PR to be useful, please consider reopening it so we can continue the discussion.

@AndrewLawendy AndrewLawendy changed the title docs(space age): 📝 introduce specific instructions and move current instructions into introduction [Space Age]: Update introduction and instructions Jul 31, 2023
@BethanyG
Copy link
Member

BethanyG commented Jul 31, 2023

@AndrewLawendy - Please see this post. This repo is currently closed to unsolicited community contributions. The one exception is for approaches articles. Since your first PR was for an approach, the bot did not close it (and I decided the information would be good in hints and appends, so went with it). However, this current PR is for a different type of change, so the bot closed it. 🙂

The instructions for practice exercises (exercises that are not the main ones on the syllabus tree) are pulled from the problem specifications repo, and are meant to be generic enough to apply to all 67 language tracks.

Direct changes to them need to be proposed in the forum, discussed, and if accepted, PR'd to that repo. Since the instructions for this exercise were quite recently updated (and most language tracks are not OOP, so wouldn't be requiring classes to complete the exercise), I don't know if there is appetite for changing them again.

@AndrewLawendy
Copy link
Contributor Author

I did read the article, but since my last pull request was successfully processed, I assumed that there might be an issue with the bot.
However, I now realize that I misunderstood the situation.
Thank you for taking the time to clarify this for me.

@BethanyG
Copy link
Member

To tag on to my previous comment - if there is something needed for instructions that is track or language specific, it can be included in a track instruction append that gets automatically merged into instructions on the website. That is what we did for your numbers information for gigasecond in the previous PR.

We might consider the "standard" "a class is needed for this exercise" append for this exercise. Something similar to this or this.

So if you'd like to re-work your information into an instruction append, I would gladly review and reopen this PR.

@AndrewLawendy
Copy link
Contributor Author

I would love to

@AndrewLawendy
Copy link
Contributor Author

@BethanyG I pushed the changes

@AndrewLawendy
Copy link
Contributor Author

AndrewLawendy commented Aug 1, 2023

On another note @BethanyG this approach is problematic

The way it returns is not ideal and should not be encouraged or taught to student, this will lead to tail recursion.
You can find an ideal way of recursion here https://exercism.org/tracks/python/exercises/collatz-conjecture/solutions/bobahop.

Do you think this is worth opening an pr for?

Reference : https://stackoverflow.com/questions/33923/what-is-tail-recursion

@BethanyG
Copy link
Member

BethanyG commented Aug 1, 2023

@AndrewLawendy -- Let's keep the discussion to one exercise/change/problem per PR/issue, please. This PR is about Space Age and not Collatz Conjecture. If you'd like to discuss the approach for Collatz Conjecture, you should open a separate PR or issue for it. 😄

Otherwise, things can get messy and hard to track. Many thanks!

@BethanyG BethanyG reopened this Aug 1, 2023
@AndrewLawendy
Copy link
Contributor Author

Hey @BethanyG , just checking if you need any change?

@BethanyG
Copy link
Member

BethanyG commented Aug 4, 2023

@AndrewLawendy - I haven't had time to review this yet, and probably won't get to it until this weekend. Please bear with me. Thanks!

Changed the wording a bit and added some links to the classes concept and the Python documentation.
@BethanyG
Copy link
Member

BethanyG commented Aug 8, 2023

@AndrewLawendy - I went ahead and added some links to the Python docs. If this all looks good to you, let me know and I will merge. Thanks!

@AndrewLawendy
Copy link
Contributor Author

@BethanyG I like what you did

@BethanyG BethanyG merged commit f7fcf6f into exercism:main Aug 8, 2023
8 checks passed
@AndrewLawendy AndrewLawendy deleted the SPACE-AGE-INSTRUCTIONS branch August 9, 2023 11:46
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.

2 participants