Skip to content

Pangram: check case insensitive behavior#1660

Merged
SleeplessByte merged 3 commits intomainfrom
1658-redo
Dec 10, 2021
Merged

Pangram: check case insensitive behavior#1660
SleeplessByte merged 3 commits intomainfrom
1658-redo

Conversation

@iHiD
Copy link
Copy Markdown
Member

@iHiD iHiD commented May 25, 2020

This is a re-opening of #1658

@iHiD iHiD added the hold label May 25, 2020
Base automatically changed from master to main January 27, 2021 15:31
@ErikSchierboom
Copy link
Copy Markdown
Member

I've rebased this on the latest main and made one change, inspired by #1658 (comment): removing the second test case.

CC @exercism/reviewers

Copy link
Copy Markdown
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

I'm not huge on the necessity of this case, because supposedly case 2577bf54-83c8-402d-a64b-a2c0f7bb213a "case insensitive" already handles it.

$ ruby -e 'p ("the quick brown fox jumps over with lazy FX".chars - [" "]).uniq.size' 
26

But, this is certainly a simpler and easier-to-understand way of expressing the same thing. Maybe it could even go before case 2577bf54-83c8-402d-a64b-a2c0f7bb213a "case insensitive" ...

Comment thread exercises/pangram/canonical-data.json Outdated
Co-authored-by: ee7 <45465154+ee7@users.noreply.github.com>
Copy link
Copy Markdown
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

I think they test the same thing. Replacing seems a fine idea.

Comment thread exercises/pangram/canonical-data.json Outdated
Co-authored-by: ee7 <45465154+ee7@users.noreply.github.com>
@SleeplessByte SleeplessByte merged commit fc50648 into main Dec 10, 2021
@SleeplessByte SleeplessByte deleted the 1658-redo branch December 10, 2021 19:14
@SleeplessByte
Copy link
Copy Markdown
Member

Thanks all!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants