Skip to content

Glasgow| 26-ITP-Jan | Alasdair MacDonald | Sprint 3 | Practice TDD#1165

Open
MacDonald91 wants to merge 3 commits intoCodeYourFuture:mainfrom
MacDonald91:coursework/sprint-3-practice-tdd
Open

Glasgow| 26-ITP-Jan | Alasdair MacDonald | Sprint 3 | Practice TDD#1165
MacDonald91 wants to merge 3 commits intoCodeYourFuture:mainfrom
MacDonald91:coursework/sprint-3-practice-tdd

Conversation

@MacDonald91
Copy link
Copy Markdown

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 all tasks in sprint 3 part TDD, the second time uploading to GitHub, a JSON file was included in the last upload; made sure this time it didn't include the file.

Questions

No questions at this time.

@MacDonald91 MacDonald91 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 4, 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 Mar 4, 2026
@github-actions

This comment has been minimized.

@MacDonald91 MacDonald91 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 4, 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 Mar 4, 2026
@MacDonald91 MacDonald91 force-pushed the coursework/sprint-3-practice-tdd branch from 273a8e0 to d4e4a58 Compare March 4, 2026 15:56
@MacDonald91 MacDonald91 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 4, 2026
Copy link
Copy Markdown
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.

Function implementation are good.

Comment thread Sprint-3/2-practice-tdd/get-ordinal-number.test.js Outdated
Comment thread Sprint-3/2-practice-tdd/count.test.js Outdated
@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 Mar 13, 2026
@MacDonald91
Copy link
Copy Markdown
Author

Thanks for the feedback!

I’ve made the following updates:

  • Removed unintended early return statements that were preventing functions from executing correctly.
  • Updated the countChar to ensure correct counting logic and added additional tests to cover case sensitivity and non-alphabet characters.
  • Improved getOrdinalNumberlogic and clarified test descriptions to better define expected behavior (especially for “th” cases).
  • Cleaned up repeatStrto remove duplicate/incorrect function definitions and ensure proper error handling for negative values.

These changes should make the functions more robust, readable, and aligned with the test expectations. Let me know if there’s anything else I can improve.

@MacDonald91 MacDonald91 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 15, 2026
@cjyuan
Copy link
Copy Markdown
Contributor

cjyuan commented Apr 15, 2026

Changes look good.

@cjyuan cjyuan 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. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 15, 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants