Skip to content

phone-number: add cases to test data that cover invalid use of "1" or "0"#1327

Merged
rpottsoh merged 3 commits intoexercism:masterfrom
LyleCharlesScott:master
Sep 19, 2018
Merged

phone-number: add cases to test data that cover invalid use of "1" or "0"#1327
rpottsoh merged 3 commits intoexercism:masterfrom
LyleCharlesScott:master

Conversation

@LyleCharlesScott
Copy link
Copy Markdown
Contributor

@LyleCharlesScott LyleCharlesScott commented Sep 13, 2018

…11-digit numbers with invalid area/exchange codes

closes #1320

…11-digit numbers with invalid area/exchange codes
@rpottsoh rpottsoh changed the title add cases to canonical exercises that cover the possibility of valid … phone-number: add cases to test data that cover invalid use of "1" Sep 13, 2018
@rpottsoh rpottsoh changed the title phone-number: add cases to test data that cover invalid use of "1" phone-number: add cases to test data that cover invalid use of "1" or "0" Sep 13, 2018
Copy link
Copy Markdown
Member

@rpottsoh rpottsoh left a comment

Choose a reason for hiding this comment

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

version needs to be bumped per guide lines in README. You may need to bump version a second time if #1325 is merged ahead of you, in which case you will need to rebase as well I think.

@LyleCharlesScott
Copy link
Copy Markdown
Contributor Author

@rpottsoh Version has been updated

Copy link
Copy Markdown
Member

@rpottsoh rpottsoh left a comment

Choose a reason for hiding this comment

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

Version not correct.

{
"exercise": "phone-number",
"version": "1.4.0",
"version": "1.4.1",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This isn't quite right. The changes you are proposing are more substantial. "1.5.0" should be the new version. Test Data Versioning tries to explain it.

@rpottsoh
Copy link
Copy Markdown
Member

I am going to go ahead and merge this seeings how there are two approvals and given that the PR has been open for 7 days.

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.

phone-number: test cases could be more thorough

3 participants