Fixes #310 Conform phone number unit test to NANP and updated track to 1.2.0 #359
Conversation
|
This is for issue: #310. Please review 😄 |
Can you confirm whether the issue is resolved if this PR is merged? If it is, we need you to use one of the words in https://help.github.com/articles/closing-issues-using-keywords/ in your commit message or your PR description so that the issue will automatically be closed. If it is not, could you describe briefly in #310 what work will still need to be done after this is merged? Thank you. |
| } | ||
|
|
||
| #[test] | ||
| #[ignore] |
There was a problem hiding this comment.
please leave the ignores in place, as discussed in #279 and https://github.com/exercism/docs/blob/master/language-tracks/exercises/anatomy/test-suites.md (where it says "In order to provide a better user experience, some language tracks only leave the first test in an exercise test suite active.")
exercises/phone-number/example.rs
Outdated
| @@ -18,10 +25,14 @@ pub fn area_code(s: &str) -> Option<String> { | |||
| } | |||
|
|
|||
| pub fn pretty_print(s: &str) -> String { | |||
There was a problem hiding this comment.
since pretty_print is no longer being tested, you can and should now remove this function
You also can/should remove area_code
vinmaster
changed the title
|
Thanks for reviewing! I've made the changes mentioned |
There was a problem hiding this comment.
Thank you, this is suitable.
There is one thing I will fix for you before merging, please do it if you choose to submit another PR: The words in https://help.github.com/articles/closing-issues-using-keywords/ need to appear in the description (the field that currently says "No description provided" if you look at #359) or the commit message, not the title as they currently are.
petertseng
added
the
sync/tests
No description provided.