Skip to content

Conversation

@alexandru-pocovnicu
Copy link

@alexandru-pocovnicu alexandru-pocovnicu commented Oct 13, 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

In this PR I implemented functions and rewrote tests for those functions using jest

Questions

in the number 3 get card value test,
test("should return 11 for Ace of Spades", () => {
const aceofSpades = getCardValue("A♠");
expect(aceofSpades).toEqual(11);
});
why is it necessary to declare a variable, can't we just write expect(getCardValue("A♠")).toEqual(11); instead?
thank you

@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: Wrong number of parts separated by |s

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).

@alexandru-pocovnicu alexandru-pocovnicu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 13, 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: Wrong number of parts separated by |s

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).

4 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: Wrong number of parts separated by |s

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: Wrong number of parts separated by |s

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: Wrong number of parts separated by |s

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: Wrong number of parts separated by |s

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).

@ckirby19
Copy link

Looking great! Just a few minor comments, mostly around consistency with spacing. Great job!

@ckirby19 ckirby19 added the Reviewed Volunteer to add when completing a review with trainee action still to take. label 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: Wrong number of parts separated by |s

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).

@ckirby19 ckirby19 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label 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: Wrong number of parts separated by |s

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).

@alexandru-pocovnicu
Copy link
Author

alexandru-pocovnicu commented Oct 20, 2025

thank you for your time @ckirby19 , can't seem to locate any of the comments

if (Math.abs(numerator) === Math.abs(denominator) ){
return false;
}
if (isNaN(numerator)|| isNaN(denominator)) {

Choose a reason for hiding this comment

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

Spacing again before the ||

@ckirby19
Copy link

ckirby19 commented Oct 20, 2025

Apologies @alexandru-pocovnicu, I sent finished my review now and sent those comments. Please let me know if you still can't see them

@alexandru-pocovnicu
Copy link
Author

all good and thank you

@alexandru-pocovnicu alexandru-pocovnicu 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 23, 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: Wrong number of parts separated by |s

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).

1 similar comment
@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: Wrong number of parts separated by |s

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).

if(numberRank>=2 && numberRank<10){
return numberRank;
}
if(rank==="10" || rank==="J" || rank==="Q" || rank==="K"){

Choose a reason for hiding this comment

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

Formatting: it is usually preferred to give equality checks with a space, so rank === "10" instead of rank==="10".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants