Skip to content

Manchester | 26-ITP-Sep | Precious Moses | Sprint 1 | FormatAs12HourClock - #1637

Open
moses77-boop wants to merge 27 commits into
CodeYourFuture:mainfrom
moses77-boop:Sprint-1
Open

moses77-boop wants to merge 27 commits into
CodeYourFuture:mainfrom
moses77-boop:Sprint-1

Conversation

@moses77-boop

@moses77-boop moses77-boop commented Oct 3, 2026 •

Copy link
Copy Markdown

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

Task code

CYF-1197

Changelist

This PR contains fixes and tests with edge-cases.

@github-actions

This comment has been minimized.

@moses77-boop moses77-boop added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 3, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 3, 2026
@moses77-boop moses77-boop added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. 📅 Sprint 1 Assigned during Sprint 1 of this module labels Oct 3, 2026
Comment on lines +1 to +11
// function formatAs12HourClock(time) {

const hours = Number(time.slice(0, 2));
// const hours = Number(time.slice(0, 2));

if (hours > 12) {
return `${hours - 12}:00 pm`;
// if (hours > 12) {
// return `${hours - 12}:00 pm`;
// }
// return `${time} am`;
// }

// export {formatAs12HourClock};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unused code should be removed to keep the codebase clean.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed.

return `${displayHour}:${minutePart}${period}`;
}
export {formatAs12HourClock};
console.log(formatAs12HourClock("08:40"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code submitted in a PR should also be free of debugging code.

@moses77-boop moses77-boop Oct 4, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the feedback!
Fixed

Comment on lines +18 to +20
if(hours < 12){
period = "am";
} else {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The spacing before { is not fully consistent.

Suggestion:

  • Look up the benefits of using a code formatter.
  • Install the Prettier extension for VS Code, then:
    • Use VS Code's Format Document feature to format your code.
    • Optionally, enable Format On Save and Format On Paste to keep your code consistently formatted.

Resource: Visual Studio Code - Formatting

Note: The formatter may not work correctly if your code contains syntax errors.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for your feedback.

I don't paste codes. I have Prettier extension installed already, not sure why it's not auto formatting, but I'll be happy to have a look and see how to configure it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed now! 👍

@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 4, 2026

@cjyuan cjyuan left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This branch should only contain modified files in the format-clock-edge-cases folder.

Some of your recent commits have modified files in some other folders. Could you revert the changes made in those folders?

Note: Don't forget to add the "Needs review" label back to a PR whenever it is ready to be re-reviewed.

Comment on lines 5 to 10
let period;
if(hours < 12){
period = "am";
}else{
period = "pm";
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could practice using the ? : operator to simplify the code on line 5-10 into one line of code.

@moses77-boop moses77-boop Oct 5, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the feedback!

Code refactored.

Comment on lines +12 to +15
let displayHour = hours % 12;
if(displayHour === 0){
displayHour = 12;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code on lines 12-15 could also be simplified.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the feedback!

Code refactored.

@@ -1,11 +1,18 @@
function formatAs12HourClock(time) {
function formatAs12HourClock(time){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Check out this coding style article about common format convention.

The current formatting is consistent but it doesn't look like the work of Prettier.

@moses77-boop moses77-boop Oct 5, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Screenshot 2026-10-05 at 01 44 16

Above attached is a screenshot of my VS Code panel, showing Prettier, and other CYF approved extensions. If it needs further assessment I'd be glad to book a session with you to work me through on how to reconfigure Prettier in my VS Code.

@moses77-boop moses77-boop 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 5, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026
@github-actions

This comment has been minimized.

@moses77-boop moses77-boop added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026
@cjyuan

cjyuan commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Changes are good. Well done!

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. 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. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. 📅 Sprint 1 Assigned during Sprint 1 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants