Skip to content

Conversation

@alexandru-pocovnicu
Copy link

@alexandru-pocovnicu alexandru-pocovnicu commented Sep 29, 2025

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

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

@alexandru-pocovnicu alexandru-pocovnicu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 29, 2025
@github-actions
Copy link

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
@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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 alexandru-pocovnicu changed the title London|ITP-25-September|Alexandru Pocovnicu|Sprint 1 Coursework London|ITP-25-September|Alexandru Pocovnicu|Sprint 1| Coursework Sep 29, 2025
@alexandru-pocovnicu alexandru-pocovnicu changed the title London|ITP-25-September|Alexandru Pocovnicu|Sprint 1| Coursework London|25-ITP-September|Alexandru Pocovnicu|Sprint 1| Coursework Oct 4, 2025
@LonMcGregor LonMcGregor added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 6, 2025
Copy link

@LonMcGregor LonMcGregor left a 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

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?

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,

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?

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?

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?

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

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 6, 2025
@LonMcGregor
Copy link

In 1-key-exercises - 4-random.js I understand the methods but don't understand why this " (maximum - minimum + 1)) + minimum " is necessary s

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.

@alexandru-pocovnicu alexandru-pocovnicu added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 20, 2025
@LonMcGregor
Copy link

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 LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 21, 2025
@alexandru-pocovnicu alexandru-pocovnicu added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants