Skip to content

Glasgow | ITP May -25 | Pandi Simatupang | Module-Structuring-and-Testing-Data | coursework/sprint-1 #649

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

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

PandiSimatupang
Copy link

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Provided answers and comments to Sprint-1

Questions

Null

@PandiSimatupang PandiSimatupang added the Needs Review Participant to add when requesting review label Jul 10, 2025
@YoanHlebarov YoanHlebarov added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Jul 12, 2025
Copy link

@YoanHlebarov YoanHlebarov left a comment

Choose a reason for hiding this comment

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

Overall you have a good understanding of how the code flows. Just fix the few comments!

const age = 33;

//reassign variable age by 1 (+1)
age = age + 1;

Choose a reason for hiding this comment

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

That would not work as we cannot reassign variables declared with const. What other ways can you think of that can allows us to do what we want?

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't aware of "const".. I need to change the variable type (to "let") so it can be reassigned.

Choose a reason for hiding this comment

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

Exactly, let would be the correct approach here. If you do not need to reassign a variable always use const. This ensures that your variable will have the information you expect it to have.

Choose a reason for hiding this comment

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

You should be aware of the term Hoisting, it would allow us to access variables declared with var before they are assigned. You can read more about it in the link. What you have done is the best approach, and var should be used only when absolutely necessary, but it is worth knowing more about it.

Copy link
Author

Choose a reason for hiding this comment

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

Ok. noted

Choose a reason for hiding this comment

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

Looks like you still need to answer this one!

Copy link
Author

Choose a reason for hiding this comment

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

function square(num) {
return num * num;
}

mynum = 3; //define the num
console.log(square(mynum)); //verify the result

Copy link
Author

Choose a reason for hiding this comment

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

I did that on Branch sprint-1.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, OK.. outdated! No worries

Choose a reason for hiding this comment

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

I think you still need to add your prediction of what will happen if we were to run the function. Then see if there is anything that needs to be fixed, and write a new function that resolves the errors and returns the expected result!

Choose a reason for hiding this comment

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

Looks like you still need to answer this one!

Copy link
Author

Choose a reason for hiding this comment

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

well, interesting.. I did that on Sprint 1 branch.

Copy link
Author

Choose a reason for hiding this comment

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

No worries

Choose a reason for hiding this comment

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

I think you would need to implement the function here on this branch (which is sprint-1 too. Have you pushed all of your changes to origin?

You should implement a function that calculates BMI based off two arguments - weight and height.

@YoanHlebarov YoanHlebarov removed the Needs Review Participant to add when requesting review label Jul 12, 2025
@YoanHlebarov YoanHlebarov added Reviewed Volunteer to add when completing a review and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jul 12, 2025
@PandiSimatupang PandiSimatupang added Needs Review Participant to add when requesting review and removed Reviewed Volunteer to add when completing a review labels Jul 16, 2025
@YoanHlebarov
Copy link

YoanHlebarov commented Jul 19, 2025

I think you still need to complete Sprint-2/1-key-errors/2.js and Sprint-2/3-mandatory-implement/1-bmi.js. If you have added the code required in another branch make sure to merge it to this one and push the changes!

Right @PandiSimatupang I noticed the issue. (sorry my bad!) These two files need to be removed as they are not related to Sprint 1!

@YoanHlebarov YoanHlebarov added 👀 Review Requirements Changes requested to meet requirements 👀 Review Git Changes requested to do with Git and removed 👀 Review Requirements Changes requested to meet requirements labels Jul 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 Review Git Changes requested to do with Git Needs Review Participant to add when requesting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants