-
-
Notifications
You must be signed in to change notification settings - Fork 239
London|25-ITP-September|Alexandru Pocovnicu|Sprint 1| Coursework #719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
London|25-ITP-September|Alexandru Pocovnicu|Sprint 1| Coursework #719
Conversation
…ressions in 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 |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is difficult to understand about the MDN docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider all the edge cases - are there any inputs that might give strange formatting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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
|
Good work so far. I've left another comment on the movie duration task, it's the only one still left to work on. |
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