-
-
Notifications
You must be signed in to change notification settings - Fork 556
pig-latin: improve documentation #1171
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
Conversation
Appears to be related to #1049 |
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 have one minor nit regarding bullet style which is the only reason I've marked requesting changes.
Generally speaking, whether or not we should merge this depends on what the point of the exercise is. Is it:
- Implementing an exercise based on an ambiguous spec but specific tests?
- Implementing an exercise based on an explicit specification?
I'll note that the former situation is more like what I'd expect to encounter from a non-technical client, but the latter may be better for early students. That said, if the purpose of this exercise is the former, these changes should not be merged. If the purpose of the exercise is the latter, they should.
I don't have a strong opinion on that topic; the scope of my change request is limited to the bullet points. I'm interested to hear what other maintainers feel on the topic.
exercises/pig-latin/description.md
Outdated
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to | ||
the end of the word. | ||
- **Rule 2**: If a word begins with a consonant sound, move it to the | ||
* **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to |
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.
Please retain -
bullets throughout instead of switching to *
bullets. Marked here only, but applies to all four rules.
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.
No problem. I can fix this, overlooked the markdown as the output looked the same.
exercises/pig-latin/description.md
Outdated
end of the word, and then add an "ay" sound to the end of the word. | ||
* **Rule 3**: If a word starts with the following consonant clusters "ch", "qu", "th", "thr", "sch" and any consonant followed by "qu" at the beginning of the word, move it to the end of the word, and then add an "ay" sound to the end of the word (e.g. "chair" -> "airchay", "square" -> "aresquay", "thread" -> "eadthray"). | ||
* **Rule 4**: If "y" comes as the beginning of the word then it must be treated as a consonant, and if it comes after a consonant cluster or is the second letter in a two letter word it must be treated as a vowel. |
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 rule appears to conflict with the added portion of Rule 1. If the intent is to make a maximally explicit ruleset, that'll need some clarification.
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.
Are you referring to Rule 4. Do you think I should integrate it into Rule 1?
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.
Yes, I'm referring to rule 4. I don't believe it needs to be integrated, but it does need to be deconflicted.
I hope I did this right. Made some edits, please have a look and let me know if you agree with the changes. Again, the goal is to make it beginner friendly and aid my students understand this exercise better by reading the README in plain English which will help them understand the Unit Tests. |
exercises/pig-latin/description.md
Outdated
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to the end of the word. Treat "xr" and "yt" at the beginning of a word as vowels (e.g. "xray" -> "xrayay", "yttria" -> "yttriaay"). | ||
- **Rule 2**: If a word begins with a consonant sound (can be made up of multiple consonants a.k.a consonant cluster), move it to the end of the word. | ||
- **Rule 3**: If a word starts any consonant followed by "qu" at the beginning of the word, move it to the end of the word, and then add an "ay" sound to the end of the word (e.g. "square" -> "aresquay"). | ||
- **Rule 4**: If a word contains a "y" after a consonant cluster or as the second letter in a two letter word treat it as a vowel by leaving in it place. |
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.
Should rules 2 and 4 have examples as well?
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.
Good point. I can add examples.
exercises/pig-latin/description.md
Outdated
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to the end of the word. Treat "xr" and "yt" at the beginning of a word as vowels (e.g. "xray" -> "xrayay", "yttria" -> "yttriaay"). | ||
- **Rule 2**: If a word begins with a consonant sound (can be made up of multiple consonants a.k.a consonant cluster), move it to the end of the word. | ||
- **Rule 3**: If a word starts any consonant followed by "qu" at the beginning of the word, move it to the end of the word, and then add an "ay" sound to the end of the word (e.g. "square" -> "aresquay"). | ||
- **Rule 4**: If a word contains a "y" after a consonant cluster or as the second letter in a two letter word treat it as a vowel by leaving in it place. | ||
|
||
There are a few more rules for edge cases, and there are regional | ||
variants too. |
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 am not convinced of the relevancy of this statement. Perhaps if it was also stated that this exercise is focused on the 4 rules described above it would increase the relevance of the statement.... I'm interested to know what other people 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.
True, as the 4 rules pretty much explain how to make the tests pass.
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.
True, as the 4 rules pretty much explain how to make the tests pass.
Or better stated, how Pig Latin works.
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.
Yes!
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 kind of like having true statements included in a problem README, even if strictly speaking they're not relevant to solving the exercise. Additional context can mean the difference between a tedious exercise and an interesting problem.
Many people grew up speaking pig latin as children, and they may notice variations between what they remember and what is tested. For example, in the pig latin of my own childhood, "skylight" would be rendered "aylightskay" or "i-lightskay", which isn't covered by the four rules as listed here. Leaving in the note about edge case and regionalisms costs nothing and is validating.
exercises/pig-latin/description.md
Outdated
end of the word, and then add an "ay" sound to the end of the word. | ||
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to the end of the word. Treat "xr" and "yt" at the beginning of a word as vowels (e.g. "xray" -> "xrayay", "yttria" -> "yttriaay"). | ||
- **Rule 2**: If a word begins with a consonant sound (can be made up of multiple consonants a.k.a consonant cluster), move it to the end of the word. | ||
- **Rule 3**: If a word starts any consonant followed by "qu" at the beginning of the word, move it to the end of the word, and then add an "ay" sound to the end of the word (e.g. "square" -> "aresquay"). |
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.
starts any
missing a word?
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.
Yes, it is. It should read "starts with any ..."
exercises/pig-latin/description.md
Outdated
the end of the word. | ||
- **Rule 2**: If a word begins with a consonant sound, move it to the | ||
end of the word, and then add an "ay" sound to the end of the word. | ||
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to the end of the word. Treat "xr" and "yt" at the beginning of a word as vowels (e.g. "xray" -> "xrayay", "yttria" -> "yttriaay"). |
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.
The inconsistency between talking about a vowel sound versus vowels causes me, a person reading this rule, to wonder why the distinction is necessary.
Very specifically: The second sentence tells me to treat xr
and yt
as vowels. However, this is insufficient information to determine whether the first sentence ("If a word begins with a vowel sound") applies.
If the first sentence reads "If a word begins a vowel, ..." XOR if the second sentence were to say "xr and yt at the beginning of a word make vowel sounds", then the terminology would be consistent and it would be clear that the first sentence is applicable to the second.
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 agree, talking about a vowel sound makes more sense. Thanks for the feedback.
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.
Not much I can add which hasn't already been covered; I'm approving now to clear out my change request from earlier. However, please do not merge before first making the changes requested by other reviewers.
exercises/pig-latin/description.md
Outdated
- **Rule 2**: If a word begins with a consonant sound, move it to the | ||
end of the word, and then add an "ay" sound to the end of the word. | ||
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to the end of the word. Treat "xr" and "yt" at the beginning of a word as vowels (e.g. "xray" -> "xrayay", "yttria" -> "yttriaay"). | ||
- **Rule 2**: If a word begins with a consonant sound (can be made up of multiple consonants a.k.a consonant cluster), move it to the end of the word. |
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'd like to split this into two sentences for clarity:
"If a word begins with a consonant sound, move it to the end of the word. Consonant sounds can be made up of multiple consonants, a.k.a. a consonant cluster."
exercises/pig-latin/description.md
Outdated
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to the end of the word. Treat "xr" and "yt" at the beginning of a word as vowels (e.g. "xray" -> "xrayay", "yttria" -> "yttriaay"). | ||
- **Rule 2**: If a word begins with a consonant sound (can be made up of multiple consonants a.k.a consonant cluster), move it to the end of the word. | ||
- **Rule 3**: If a word starts any consonant followed by "qu" at the beginning of the word, move it to the end of the word, and then add an "ay" sound to the end of the word (e.g. "square" -> "aresquay"). | ||
- **Rule 4**: If a word contains a "y" after a consonant cluster or as the second letter in a two letter word treat it as a vowel by leaving in it place. | ||
|
||
There are a few more rules for edge cases, and there are regional | ||
variants too. |
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 kind of like having true statements included in a problem README, even if strictly speaking they're not relevant to solving the exercise. Additional context can mean the difference between a tedious exercise and an interesting problem.
Many people grew up speaking pig latin as children, and they may notice variations between what they remember and what is tested. For example, in the pig latin of my own childhood, "skylight" would be rendered "aylightskay" or "i-lightskay", which isn't covered by the four rules as listed here. Leaving in the note about edge case and regionalisms costs nothing and is validating.
Third time's the charm. I appreciate the feedback. |
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.
OK, I would consider this OK to merge as is. If desired, we could discuss some disambiguation of a few "it"s before that.
end of the word, and then add an "ay" sound to the end of the word. | ||
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to the end of the word. Please note that "xr" and "yt" at the beginning of a word make vowel sounds (e.g. "xray" -> "xrayay", "yttria" -> "yttriaay"). | ||
- **Rule 2**: If a word begins with a consonant sound, move it to the end of the word and then add an "ay" sound to the end of the word. Consonant sounds can be made up of multiple consonants, a.k.a. a consonant cluster (e.g. "chair" -> "airchay"). | ||
- **Rule 3**: If a word starts with a consonant sound followed by "qu", move it to the end of the word, and then add an "ay" sound to the end of the word (e.g. "square" -> "aresquay"). |
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.
the "it" of "move it to the end of the word" may be unclear as it refers to the combination of the consonant plus qu, but the example makes it clear enough
- **Rule 1**: If a word begins with a vowel sound, add an "ay" sound to the end of the word. Please note that "xr" and "yt" at the beginning of a word make vowel sounds (e.g. "xray" -> "xrayay", "yttria" -> "yttriaay"). | ||
- **Rule 2**: If a word begins with a consonant sound, move it to the end of the word and then add an "ay" sound to the end of the word. Consonant sounds can be made up of multiple consonants, a.k.a. a consonant cluster (e.g. "chair" -> "airchay"). | ||
- **Rule 3**: If a word starts with a consonant sound followed by "qu", move it to the end of the word, and then add an "ay" sound to the end of the word (e.g. "square" -> "aresquay"). | ||
- **Rule 4**: If a word contains a "y" after a consonant cluster or as the second letter in a two letter word it makes a vowel sound (e.g. "rhythm" -> "ythmrhay", "my" -> "ymay"). |
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.
the "it" here is probably clear enough from context; without context there are multiple things it may refer to.
Are there any proposed changes or will this be merged soon? Thank you. |
@navarrorc Depends on you: @petertseng offered some optional suggestions. If you choose to take them, there'll be another commit. If not, we can't know your intentions unless you state them explicitly. |
@navarrorc there are a couple optional suggestions made by @petertseng. Are you intending to make more changes? For the most part this PR appears ready to merge, it is just lacking the go ahead from you. Thanks for making the changes you've made thus far. |
I would suggest you merge it as it makes sense to my students. Had quite a few complete the challenge on their own with these rules. Thanks. |
Thanks @navarrorc for the feedback. This should be merged fairly soon. It will take time for the changes to then propagate to the various tracks that provide this exercise. Thanks again for your help in improving this exercise for your students and for all the students of Exercism! 👍 |
The update was discussed in the problem-specifications repo in pr exercism#1171: pig-latin improve documentation exercism/problem-specifications#1171
The new rules add additional detail. The update was discussed in the problem-specifications repo in PR #1171: pig-latin improve documentation exercism/problem-specifications#1171 Resolves #453
I'm a programming instructor and the current rules of this exercise did not provide enough detail for my students to really understand what is expected of the code they need write. Reached out to the Gitter exercism support channel and @kotp suggested I contribute to the documentation.