Manchester | 26-ITP-May | Monsur Abdulrahman | Sprint 3 | Coursework/sprint 3 | Practice TDD - #1599
Manchester | 26-ITP-May | Monsur Abdulrahman | Sprint 3 | Coursework/sprint 3 | Practice TDD#1599Monsur0001 wants to merge 4 commits into
Conversation
Currently they're the wrong way around, which isn't the point of the exercise.
…ce string repetition tests
…peatStr function to avoid using String.prototype.repeat
This comment has been minimized.
This comment has been minimized.
Monsur0001
added
📅 Sprint 3
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
Monsur0001
added
NotCoursework
Needs Review
cjyuan
added
Review in progress
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Code and tests are good. However, the PR was not properly prepared.
-
Three of the modified files are not related to the TDD exercise, please revert the changes made to them.
-
The PR description was not properly prepared. Please read this Guideline and then update the PR description accordingly.
| }); | ||
|
|
||
| // Case 5: All other numbers | ||
| test("should append 'th' for all other numbers", () => { |
There was a problem hiding this comment.
When a test fails with the message "... all other numbers", it may be unclear what "other numbers" actually refers to. Could you revise the test description to make it more informative?
| // Your implementation of this function must *not* call String.prototype.repeat (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/repeat). | ||
| // The goal is to re-implement that function, not to use it. | ||
| return "hellohellohello"; | ||
| if (count < 0) { | ||
| throw new Error("Count must be a non-negative integer"); | ||
| } | ||
| return str.repeat(count); |
There was a problem hiding this comment.
Could you rewrite this statement without using .repeat() (to meet the requirement stated on line 3)?
cjyuan
added
Reviewed
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
2 similar comments
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
3 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer. You must remove this section if you have no questions.