London | 26-ITP-Sep | Ebrahim Moqbel | Sprint 3 | coursework CYF-1053 - #1468
Open
Ebrahim-Moqbel wants to merge 27 commits into
Open
Ebrahim-Moqbel wants to merge 27 commits into
Ebrahim-Moqbel wants to merge 27 commits into
Conversation
… shirt color and changed th name to match the ID attribute.
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Ebrahim-Moqbel
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.
Ebrahim-Moqbel
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
webmonch
added
the
Reviewed
Clarified explanation of the syntax error in the comments.
Ebrahim-Moqbel
added
Needs Review
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
selyafi
reviewed
Sep 17, 2026
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
Contributor
There was a problem hiding this comment.
selyafi left ten comments on this on 17 September, and they are all still open. The label was left on Needs Review afterwards, so it may not have been obvious that a review had come in.
Have a read through his comments, push your changes, then add the Needs Review label again. I am setting this to Reviewed for now so it is not sitting in the queue as though nobody has looked at it.
Two more while you are in there:
-
Prettier on your Sprint 3 files, same as on your Sprint 2 PR. Nine of the ten fail the check at the moment.
-
1-key-errors/2.jsline 17, see my comment.
Add the Needs Review label again when you have pushed.
abdishakoor-dev
added
Reviewed
Ebrahim-Moqbel
added
Needs Review
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
Contributor
There was a problem hiding this comment.
Good progress, most of selyafi's list is done. Five things still open:
1-bmi.js: it returns a string now, but trycalculateBMI(81, 1.8). See line 19.1-key-errors/2.jsline 17: the sentence aboutnumis still in the file. You explained it back to me correctly in the thread, it just needs deleting.3-to-pounds.jsline 1: selyafi asked you to put the original task line back and it is still the changed version.time-format.jsline 40: your explanation underneath is right but the answer on the line above still says 1.- Prettier: nine of the ten files still fail. Same as on your Sprint 2 PR, so worth turning on format on save rather than doing it by hand each time.
Add the Needs Review label again once you have pushed.
| function calculateBMI(weight, height) { | ||
| // return the BMI of someone based off their weight and height | ||
| let result = (weight/(height*height)); | ||
| result= Number(result.toFixed(1)); |
Contributor
There was a problem hiding this comment.
Run calculateBMI(81, 1.8). The task asks for a string to one decimal place, and you get "25". What does Number(...) do to "25.0" before String(...) ever sees it? toFixed(1) already hands you a string.
| @@ -1,6 +1,27 @@ | |||
| // In Sprint-1, there is a program written in 3-mandatory-interpret/3-to-pounds.js | |||
| // In Sprint-1, there is a program written in interpret/to-pounds.js | |||
Contributor
There was a problem hiding this comment.
Still the edited version. The original line is // In Sprint-1, there is a program written in 3-mandatory-interpret/3-to-pounds.js.
|
|
||
| // e) What is the return value of pad when it is called for the last time in this program? Explain your answer | ||
| // =============> write your answer here | ||
| // =============> 1 |
Contributor
There was a problem hiding this comment.
What does pad(1) hand back, and what type is it? Your sentence underneath gets there, this line should match it.
|
|
||
| console.log(`The result of multiplying 10 and 32 is ${multiply(10, 32)}`); | ||
|
|
||
| console.log(multiply(10,32)) No newline at end of file |
Contributor
There was a problem hiding this comment.
Leftover from testing, this one can go. Not a blocker.
abdishakoor-dev
added
Reviewed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Learners, PR Template
Self checklist
Task code
CYF-1053
Changelist
completed all the requirements and specifications for sprint 3 files
Questions
N/A