Skip to content

Conversation

@Abayie
Copy link

@Abayie Abayie commented Nov 23, 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

Implement and rewrite test sprint 3

Questions

No questions

@Abayie Abayie added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 23, 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 missed updating two Jest test scripts in the rewrite-tests-with-jest subfolder.

Comment on lines 18 to 21
const num = Number(rank);
if (!isNaN(num) && num >= 2 && num <= 9) {
return num;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In JavaScript, strings that represent valid numeric literals in the language can be safely
converted to equivalent numbers or parsed into a valid integers.

To find out what these strings are, you can ask AI

What kinds of string values would make Number(rank) evaluate to 2 in JS?

or

What string values could make the expression !isNaN(num) && num >= 2 && num <= 9 evalute to true?

Do you want to recognize these string values as valid ranks?

@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 Nov 24, 2025
@Abayie Abayie added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 24, 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.

The tests look good.

Can you also address my comment which I left on Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 25, 2025
@Abayie
Copy link
Author

Abayie commented Nov 25, 2025

Yes, I did the research, ChatGPT made understand that are several other numbers that would evaluate to "0-10" which doesn't necessarily have to be exactly "0-10" because JavaScript is VERY permissive. It doesn’t only accept "specific number". It accepts anything that coerces to a specific number. Possible errors could be accidental "2.0", "2e0" bugs, whitespace tricks, coercion abuse

@Abayie Abayie added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 25, 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.

All good! Well done!

@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 Nov 25, 2025
@Abayie
Copy link
Author

Abayie commented Nov 26, 2025

Thank you for your guidance

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