Skip to content

Conversation

@zilinskyte
Copy link

@zilinskyte zilinskyte commented Oct 20, 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

Errors predicted, detected and fixed
Function implementation
Code interpreted and following questions answered

Questions

@github-actions
Copy link

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@zilinskyte zilinskyte changed the title West Midlands | West Midlands | 25-Sept-ITP | Georgina Rogers | Sprint-2 | Structing and Testing Data Oct 20, 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: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

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: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@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: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@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: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@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: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@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: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

@zilinskyte zilinskyte changed the title West Midlands | 25-Sept-ITP | Georgina Rogers | Sprint-2 | Structing and Testing Data West Midlands | 25-Sept-ITP | Georgina Rogers | Sprint 2 | Structing and Testing Data Oct 20, 2025
@zilinskyte zilinskyte added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Structuring-And-Testing-Data The name of the module. labels Oct 22, 2025
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

You have added a bunch of unrelated files zilinskyte_Module-Structuring-and-Testing-Data_ ITP 2_files... to this branch. Can you remove them from this branch to keep it clean?

function calculateBMI(weight, height) {
// return the BMI of someone based off their weight and height
}
var bmi = (weight / (height * height));
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using var to declare variables. Use let instead.

// return the BMI of someone based off their weight and height
}
var bmi = (weight / (height * height));
return bmi.toFixed(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

What type of value do you expect the function to return? A number or a string?
Does your function return the type of value you expect?

Comment on lines +6 to +25
function toPounds(penceString) {
penceString = String(penceString);// ensures input is treated as a string even if number is inputted
// const penceStringWithoutTrailingP = penceString.substring(
let penceStringWithoutTrailingP = penceString;
if (penceString.endsWith("p")) {
penceStringWithoutTrailingP = penceString.substring(0, penceString.length - 1);
} // This prevents the last characters from being sliced off if there is no p.

const paddedPenceNumberString = penceStringWithoutTrailingP.padStart(3, "0");
const pounds = paddedPenceNumberString.substring(
0,
paddedPenceNumberString.length - 2
);

const pence = paddedPenceNumberString
.substring(paddedPenceNumberString.length - 2)
.padEnd(2, "0");

return${pounds}.${pence}`;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you improve the indentation of the code in this function?

Note: In VSCode, we can install the prettier extension and then use the "Format document" function" to automatically indent code.

if (hours > 12) {
return `${hours - 12}:00 pm`;
}
const minutes = time.slice(3);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also consider use time.slice(-2) to better express we are extracting the last two characters.

Comment on lines +15 to 17
if (hours > 12) return `${hours - 12}:00 pm`;

return `${time} am`;
Copy link
Contributor

Choose a reason for hiding this comment

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

The return values of these two function calls are not formatted consistently.

console.log(formatAs12HourClock("15:30"));
console.log(formatAs12HourClock("03:30"));

Can you improve the consistency?

@cjyuan cjyuan 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 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Structuring-And-Testing-Data The name of the module. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants