Module-Structuring-and-Testing-Data | Sprint-1 #528
Conversation
add logic to get dir and ext parts of file path
add random number min and max; add step by step comment; add console output
fix TypeError
add answers to all questions;correct syntax error in replaceALL()
detailed explanation
add explanation for alert and prompt function in chrome console
explanation of console object and its methods in Chrome Devtools
LonMcGregor
added
Review in progress
There was a problem hiding this comment.
You've made some good progress solving all of this. I've left a few questions throughout the comments made in this pull request, if you could please take a look and answer them.
When making a pull request please try to keep them specific to what you're implementing. In an organisation this might be one specific feature. Here we are looking for one sprint at a time, and you've submitted two at once here.
Also, make sure you understand what the template in the pull request is asking - you ticked the box to say you followed the title format, but it's missing your name and region. And some of the code does not run.
Good work, but a few things still to work on.
| @@ -20,4 +20,11 @@ console.log(`The base part of ${filePath} is ${base}`); | |||
| const dir = ; | |||
| const ext = ; | |||
|
|
|||
There was a problem hiding this comment.
Be careful about reassigning variables - you've reused the dir and ext constants here
There was a problem hiding this comment.
can you explain the change you made here?
There was a problem hiding this comment.
You don't need to commit this file - do you remember how you can use .gitignore?
There was a problem hiding this comment.
Good answer, I think you mixed up question d) with e) and are missing an answer. Could you identify the variable declarations?
| @@ -5,16 +5,28 @@ | |||
|
|
|||
| // =============> write your prediction of the error here | |||
|
|
|||
| //we will get an error bcs the parameter of the function need to string type not number so it can be something like num | |||
|
|
|||
There was a problem hiding this comment.
Could you explain what you mean by the parameter needing to be a string type?
| squareHeight =height * height; | ||
| bmi = weight / squareHeight; | ||
| return bmi.0toFixed(1); | ||
|
|
There was a problem hiding this comment.
I think there is a typo in this function, can you fix it?
There was a problem hiding this comment.
Good solution. You could probably simplify what you are returning. Do you need to assign it to a variable before returning?
There was a problem hiding this comment.
This is a good answer for solving a problem with weight, but I think the question is asking about currency, like in Sprint-1/3-mandatory-interpret/3-to-pounds.js
Can you write a toPounds function that takes an input like "399p" and turns it into "£3.99"?
LonMcGregor
added
Reviewed
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.