London|25-ITP-September|Alexandru Pocovnicu|Sprint 1| Coursework#719
London|25-ITP-September|Alexandru Pocovnicu|Sprint 1| Coursework#719alexandru-pocovnicu wants to merge 20 commits intoCodeYourFuture:mainfrom
Conversation
…ressions in comments
alexandru-pocovnicu
added
the
Needs Review
|
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 |
5 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 |
|
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 |
|
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 |
|
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 |
|
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 |
alexandru-pocovnicu
changed the title
alexandru-pocovnicu
changed the title
LonMcGregor
added
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start on this sprint's tasks, I have spotted a few areas where you could improve code further, or things you might want to discuss
|
|
||
| // https://www.google.com/search?q=get+first+character+of+string+mdn | ||
|
|
||
| // I really can't understand any of the documentation from developer.mozilla No newline at end of file |
There was a problem hiding this comment.
What is difficult to understand about the MDN docs?
There was a problem hiding this comment.
I can't really point to one thing which I don't understand, in the end I just copy/paste into chat gpt and ask it to explain
| //"Movie length" | ||
|
|
||
| // f) Try experimenting with different values of movieLength. Will this code work for all values of movieLength? Explain your answer | ||
| //it will work with all values as long as they are numbers, |
There was a problem hiding this comment.
consider all the edge cases - are there any inputs that might give strange formatting?
There was a problem hiding this comment.
i have tried from 0 to 5464557657, it seems to be working, is there something i'm not seeing?
There was a problem hiding this comment.
The formatting is trying to be HH:MM:SS. Are there any cases where it doesn't give a format exactly like HH:MM:SS?
Because this variable is formatted like HH:MM:SS, if a developer looked at it, would the suggested variable name const totalMovieTime accurately tell me that it's formatted like this?
There was a problem hiding this comment.
i added "0" as padding and changed the name to timeInHhMmSs, but to answer your first question if the hours would be in the 100s then the format would be HHH... etc
LonMcGregor
added
Reviewed
If you want to investigate this, it might be helpful to try running the code yourself, but changing this (e.g. removing some parts) to see what, if anything, stops working. |
…t for reassignment
alexandru-pocovnicu
added
Needs Review
|
Good work so far. I've left another comment on the movie duration task, it's the only one still left to work on. |
LonMcGregor
added
Reviewed
alexandru-pocovnicu
added
Needs Review
|
Good work on this sprint |
LonMcGregor
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
Completed Sprint 1 exercises in which i added comments, fixed errors,and interpreted code
Questions
In 1-key-exercises - 4-random.js I understand the methods but don't understand why this " (maximum - minimum + 1)) + minimum " is necessary s