Skip to content

NW | 25-ITP-Sep | TzeMing Ho | Sprint 1 | coursework/sprint-1 #706

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

NW | 25-ITP-Sep | TzeMing Ho | Sprint 1 | coursework/sprint-1 #706

wants to merge 18 commits into from

Conversation

TzeMingHo
Copy link

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

I have completed the 4 exercises in Sprint-1, including answering questions, fixing errors, interpreting, and exploring.

@TzeMingHo TzeMingHo added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 16, 2025
@jenny-alexander jenny-alexander self-requested a review September 17, 2025 12:55
@jenny-alexander jenny-alexander 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 Sep 17, 2025
@jenny-alexander
Copy link

You show a very good understanding of variables, functions, and general programming syntax. Nice work! I left a few comments for you to respond to.

@jenny-alexander jenny-alexander 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 Sep 20, 2025
@TzeMingHo TzeMingHo added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 21, 2025
@TzeMingHo
Copy link
Author

Hi Jenny,

Thank you for reviewing my pull request.
I am happy that you left a few comments for me.
However, I am having difficulty locating the comments. I have checked both the conversation section and the files changed section, but still can't find your comments.
Could you please provide me with a guideline on where I can find the comments?

jenny-alexander
jenny-alexander suggested changes Sep 21, 2025
View reviewed changes
Sprint-1/1-key-exercises/4-random.js Outdated
// Math.floor - Rounds a number DOWN to the nearest integer
// Math.random - generates a random floating number > 0 and < 1
// (maximum - minimum + 1) = 100 - 1 + 1 = 100
// So, num represents a rounded down random integer generated between 1 and 100 + 1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job explaining. 🤔 Is the "+1" at the end of line 14 a typo?
num will be a number between 1-100 (inclusive)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jenny,

You are right. I was not thinking of even the edge case, as Math.floor(99.99) + 1 = 99 + 1 = 100, has been rounded down. So, the num should have been between 1 and 100 inclusive.
I have updated the answer in 0da267c.

Sprint-1/3-mandatory-interpret/1-percentage-change.js

// Read the code and then answer the questions below

// a) How many function calls are there in this file? Write down all the lines where a function call is made
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing one function call in your list. Can you find it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, I should have included the console.log function in the list. The console.log function is a built-in function that can also be called. I have updated the answer in 0da267c.

@jenny-alexander
Copy link

Hi Jenny,

Thank you for reviewing my pull request. I am happy that you left a few comments for me. However, I am having difficulty locating the comments. I have checked both the conversation section and the files changed section, but still can't find your comments. Could you please provide me with a guideline on where I can find the comments?

@TzeMingHo oh oops! I forgot to 'submit' my comments yesterday.. Sorry about that. Hopefully you can see them now?

@jenny-alexander jenny-alexander removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 21, 2025
@TzeMingHo TzeMingHo added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 22, 2025
@jenny-alexander
Copy link

Great job @TzeMingHo 🙌

@jenny-alexander jenny-alexander added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Sep 22, 2025
@TzeMingHo TzeMingHo added the 📅 Sprint 1 Assigned during Sprint 1 of this module label Sep 22, 2025
@TzeMingHo TzeMingHo mentioned this pull request Sep 26, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jenny-alexander jenny-alexander jenny-alexander requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 1 Assigned during Sprint 1 of this module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@TzeMingHo @jenny-alexander