kindergarten-garden: Remove tests not specified in the description #930
Conversation
| @@ -56,3 +56,6 @@ Then if asked for Alice's plants, it should provide: | |||
| While asking for Bob's plants would yield: | |||
|
|
|||
| - Clover, grass, clover, clover | |||
|
|
|||
|
|
|||
| **YOU MAY ALSO NEED TO REPORT ON OTHER CLASSES WITH DIFFERENT STUDENTS.** | |||
There was a problem hiding this comment.
This feels to me like someone shouting orders at me.
Could this perhaps be worded in a more friendly way?
There was a problem hiding this comment.
Should I reduce the emphasis by removing an asterisk?
There was a problem hiding this comment.
I think it should explain how the sub gardens are associated with the other names... And not just tell me that names may be different.
The description says we have a to m, but now all of a sudden you tell me I need to know the answer for z...
There was a problem hiding this comment.
@mhdsmr2 you added the sentence that there might pop a set of different names may pop up, so it's your task to explain...
There was a problem hiding this comment.
I do not think that the less emphased version is any better…
It still leaves many questions open.
|
Because of this PR I took a brief look into the discussion in #659, also I took a look at the exercises description and test-data. In my opinion those do not fit together, therefore I've left a huge comment with 2 proposals in it. |
|
Edited for grammar - @Insti |
Insti
changed the title
|
Did changes according to comments in #659. |
| @@ -55,4 +55,4 @@ Then if asked for Alice's plants, it should provide: | |||
|
|
|||
| While asking for Bob's plants would yield: | |||
|
|
|||
| - Clover, grass, clover, clover | |||
| - Clover, grass, clover, clover | |||
There was a problem hiding this comment.
What has changed here? Is this an end of file newline issue?
There was a problem hiding this comment.
The description file should not be changed by this PR.
Try checking out a clean version of this file.
Remove name of the students from canonical-data file which are not mentioned in README
|
Did the changes. Sorry for the stupid mistake. |
Insti
changed the title
|
I think I have done all the work now. Do I need to do anything or the PR is ready to merge? |
|
We need to wait for a few more people to look at it and see if they see anything that they think needs changing. If they approve then it can be merged. |
|
Oh, okay. |
|
@NobbZ you should also have a look at it. |
|
Great!! :) What about @kytrinyx 's thought on it? |
|
@rpottsoh Oh, you can tell me the mistake. I'll commit that to my repo so there is less PRs. It's okay if you don't want to. |
|
@mhdsmr2 I admire your enthusiasm, but those changes shouldn't be included in this PR. I have already submitted the PR. |
|
Okay, no problem. |
|
Don't you guys think that it's ready to merge? |
|
@mhdsmr2 have patience. Thank you so much |
|
Thank you, Katrina. |
…1222) closes #1213 #930 removes tests that have diagram and students. All gardens are created with the diagram only. Signed-off-by: Matthew Roseman <[email protected]>
|
No wonder I missed this update; this should have chnaged the version number |
Adds a disclaimer in Kindergarten-garden that names of students may differ.
Edit:
There has been further discussion about what needs to happen to this problem so this PR has changed from its initial version.
The current proposal is to remove the tests for different student names. These are not specified in the description which is otherwise explicit about the names and ordering of students in the class.
Closes #659