Manchester| ITP-Jan-26 | Ofonime Edak | Sprint 1| Structuring and Testing Data#1052
Manchester| ITP-Jan-26 | Ofonime Edak | Sprint 1| Structuring and Testing Data#1052Ofonimeedak wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
Ofonimeedak
added
Needs Review
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.
Ofonimeedak
added
📅 Sprint 1
This comment has been minimized.
This comment has been minimized.
1 similar comment
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.
Ofonimeedak
added
the
Needs Review
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.
Ofonimeedak
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
I noticed some inconsistent formatting in the code.
Consider install prettier VSCode extension and enable formatting on save/paste on VSCode as recommended in
https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data/blob/main/readme.md
|
|
||
| // e) What do you think the variable result represents? Can you think of a better name for this variable? | ||
| // the variable result represent total movie duration in hours,minute and seconds. | ||
| // const=movieLength |
There was a problem hiding this comment.
The name movieLength does not quite indicate the value stored in the variable
is a formatted string in the form "2:12:02".
Can you suggest a more descriptive name?
| // 5.const pence = paddedPenceNumberString .substring(paddedPenceNumberString.length - 2).padEnd(2, "0");: This expression add '0' to the end if the paddedString is not | ||
| // upto 2 characters |
There was a problem hiding this comment.
Optional challenge:
Could we expect this program to work as intended for any valid penceString if we deleted .padEnd(2, "0") from the code?
In other words, do we really need .padEnd(2, "0") in this script?
There was a problem hiding this comment.
We don't really need the .padEnd() at the end, as it helps only when the length of the string is less than 3
There was a problem hiding this comment.
How do you know if paddedPenceNumberString is guaranteed to have at least 3 character?
| What is the return value of `prompt`? | ||
| it returns the value i entered "Edak" |
There was a problem hiding this comment.
What would the function return if you entered "Edak" and clicked "Cancel" instead of "OK"?
| console.log: Output a message to the console | ||
|
|
||
| console.assert: Log an error message to the console if the first argument is false. | ||
| `.` console is the class the . helps you to access the methods or properties of the class console. |
There was a problem hiding this comment.
console is not a class. Can you look up the term that refers to the type of value console stores?
cjyuan
added
Reviewed
Ofonimeedak
added
the
Needs Review
|
The changed files in this PR don't match what is expected for this task. Please check that you committed the right files for the task, and that there are no accidentally committed files from other sprints. Please review the changed files tab at the top of the page, we are only expecting changes in this directory: 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. |
github-actions
bot
removed
the
Needs Review
There was a problem hiding this comment.
I still see some inconsistent formatted code.
Have you installed prettier VSCode extension and enabled formatting on save/paste on VSCode as recommended in
https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data/blob/main/readme.md
?
| console.log: Output a message to the console | ||
|
|
||
| console.assert: Log an error message to the console if the first argument is false. | ||
| `.` the console stored methods. No newline at end of file |
There was a problem hiding this comment.
. is an operator. Can you look up the name of this operator and find out what it does?
Sprint-3/2-practice-tdd/count.js
Outdated
There was a problem hiding this comment.
This file is not related to Sprint-1 exercise. If you don't revert (undo) the change, the validation bot will keep complaining and prevent you from using the "Needs Review" label.
One way to revert changes to a file is via the git restore command.
First, locate a commit before the file was modified. It could be the first commit you made, assuming the branch started in a clean state.
Next, record the first 7 characters of the commit SHA.
Suppose the file is path/to/file, and you want to restore the file to its state before the commit with SHA 1234567.
The command is:
git restore --source=1234567^ path/to/file
After the file is restored, make a commit and push the changes to GitHub.
| // the variable result represent total movie duration in hours,minute and seconds. | ||
| // const=movieDuration |
There was a problem hiding this comment.
The name movieDuration still does not quite indicate the value stored in the variable
is a formatted string in the form "2:12:02". For examples, 1.5 (hours) and 90 (minutes) are valid movie duration too.
One more try for a more descriptive name?
Ofonimeedak
added
the
Needs Review
|
The changed files in this PR don't match what is expected for this task. Please check that you committed the right files for the task, and that there are no accidentally committed files from other sprints. Please review the changed files tab at the top of the page, we are only expecting changes in this directory: 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. |
github-actions
bot
removed
the
Needs Review
Ofonimeedak
force-pushed
the
coursework/sprint-1
branch
from
af8387c to
63c55f5
Compare
Ofonimeedak
added
the
Needs Review
|
The changed files in this PR don't match what is expected for this task. Please check that you committed the right files for the task, and that there are no accidentally committed files from other sprints. Please review the changed files tab at the top of the page, we are only expecting changes in this directory: 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. |
github-actions
bot
removed
the
Needs Review
Ofonimeedak
added
the
Needs Review
|
The changed files in this PR don't match what is expected for this task. Please check that you committed the right files for the task, and that there are no accidentally committed files from other sprints. Please review the changed files tab at the top of the page, we are only expecting changes in this directory: 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. |
github-actions
bot
removed
the
Needs Review
Ofonimeedak
added
the
Needs Review
|
The changed files in this PR don't match what is expected for this task. Please check that you committed the right files for the task, and that there are no accidentally committed files from other sprints. Please review the changed files tab at the top of the page, we are only expecting changes in this directory: 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. |
github-actions
bot
removed
the
Needs Review
Ofonimeedak
added
the
Needs Review
|
The changed files in this PR don't match what is expected for this task. Please check that you committed the right files for the task, and that there are no accidentally committed files from other sprints. Please review the changed files tab at the top of the page, we are only expecting changes in this directory: 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. |
github-actions
bot
removed
the
Needs Review
|
closing this PR because of a messy branch, all correction effected, is been carried over to the new PR |
|
No problem. |
2 participants
Self checklist
Changelist
Made changes to each of the folders and tried solving the problems in all categories
Coursework solution on structuring and testing data, sprint 1. It outlines steps and solutions for 1-key-exercise, mandatory-errors, mandatory-interprete, stretch-explore, and the readme.md problems