curriculum
curriculum copied to clipboard
TheOdinProject
Rewrite "rock paper scissors" project
Because
This PR
- Improve clarity, wording and punctuation of the "rock paper scissors"
Issue
Closes #XXXXX
Additional Information
Pull Request Requirements
- [x] I have thoroughly read and understand The Odin Project Contributing Guide
- [x] The title of this PR follows the
location of change: brief description of changeformat, e.g.Intro to HTML and CSS lesson: Fix link text - [x] The
Becausesection summarizes the reason for this PR - [x] The
This PRsection has a bullet point list describing the changes in this PR - [x] If this PR addresses an open issue, it is linked in the
Issuesection - [x] If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
- [x] If any lesson files are included in this PR, they follow the Layout Style Guide
Sidenote: You can leave the resolving up to the reviewer usually. That way they can check and resolve their requests for changes once they are committed.
I'll check back tomorrow for it's late here. Thanks for taking this on!
I've made significant changes to the original, including the requested changes. I'm unsure about using tip note boxes on the assignment; tell me what you think.
Thanks @cakegod !
Those changes are a lot more than expected. I can't review them today anymore. I don't think the tips should be in lesson note elements no. They drag too much attention in my opinion with the added color and I believe I liked the previous ones better.
I'll review this lesson once I get some time. Thanks again!
Indeed, this is also my overall feeling.
We could probably have a specific tip note-box-like for the assignment box. Adding tips requires us to add them as bullet points, which is odd since they are not strictly part of the assignment.
That was a lot to review!
I do think this lesson was in need of some clarification judging by the amount of questions surrounding the wording in our discord.
For big changes or full lesson rewrites like this we usually start with an issue first so discussion can happen there to flesh out the exact details regarding areas of improvement. I expect most of my comments could use some discussing as well, and I welcome your input.
I also would like another maintainer to review this after we have gone through most since it encompasses a full rewrite at this point.
I may have missed things so please go through it carefully but my key points are this:
- the different -- lines between sentences are not what we normally use so I removed them
- code elements reserved for code and not for boldening or other emphasis
- the numbered list should be fully encompassing what the step is. I approached it as: could someone experienced code this without the added steps (not numbered) below?
- the steps below are breaking the problem in the numbered list item down.
Let me know what you think!
The initial idea was to fix punctuation and make minor improvements. It then evolved into something bigger, sorry!
I like the em dashes; they emphasize certain parts. I used them since I saw nothing in the layout style guide forbidding their use.
In a way, the multiple steps are mixed into each step in the current assignment, so I split them into sub-steps.
The initial idea was to fix punctuation and make minor improvements. It then evolved into something bigger, sorry!
It happens! It just takes a lot more resources than the initial PR you filed. I do think this project could be enhanced and think you put in a great step to doing so!
I like the em dashes; they emphasize certain parts. I used them since I saw nothing in the layout style guide forbidding their use.
They are not mentioned but also stand out for they are not generally used and I think the sentences don't lose a lot by not inserting them here.
In a way, the multiple steps are mixed into each step in the current assignment, so I split them into sub-steps.
Yes I can see that, and I left the sub-steps. I do think the numbering might need to go for those. And I rewrote the main steps a bit. The substeps are still an elaboration for those who need it. The only thing I wondered while reviewing is if it now doesn't go overboard on handholding, but judging from the amount of issues people have with this project being the first JS one, I think it will be a hard project either way.
Yes I can see that, and I left the sub-steps. I do think the numbering might need to go for those. And I rewrote the main steps a bit. The substeps are still an elaboration for those who need it. The only thing I wondered while reviewing is if it now doesn't go overboard on handholding, but judging from the amount of issues people have with this project being the first JS one, I think it will be a hard project either way.
The current assignment already handholds. We're simply making the assignment clearer by breaking the steps into sub-steps. :wink:
I'll commit your changes and re-check everything.
The current assignment already handholds. We're simply making the assignment clearer by breaking the steps into sub-steps. 😉
I can't say I disagree, it's just more obfuscated by it all being crammed in a sentence I guess 😅
@cakegod I have converted this lesson to draft for now so people can see it's still a work in progress.
@cakegod what's the status on this? If you're ready for a review again after making changes, there's a button to request it.
@cakegod what's the status on this? If you're ready for a review again after making changes, there's a button to request it.
Still a work in progress. I have started creating a checklist of pain points, but I have yet to write fixes for them. I'll see if I can finish it this week.
@cakegod what's the status on this? If you're ready for a review again after making changes, there's a button to request it.
Still a work in progress. I have started creating a checklist of pain points, but I have yet to write fixes for them. I'll see if I can finish it this week.
Thanks for the update. There's no rush whatsoever! I just wanted to make sure you weren't waiting for a review or anything.
It's time to finish this before I forget again. Tell me what you think about the latest changes. I'm not sure if we should add more tips or leave it as it is.
I wanted to let you know it will be a while before I get to sit down for this. My focus is on an internship currently and I expect to get back to maintaining at the start of the new year. Thanks for the work you've put in!
@cakegod Mind fixing up the remaining HTML blank lines lint errors? You can ignore the "unchanged files with check annotations" thing, just annoying noise from Github's beta feature.
Then once the blank line lint errors are gone, if @thatblindgeye doesn't have any further change requests, we could get this merged?
I am not sure if we want to include validation on the human player input, as it would require a loop or recursion. :thinking:
I am not sure if we want to include validation on the human player input, as it would require a loop or recursion. 🤔
I'd say no need for validation for that exact reason.