Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add tertiary phone number #102

Merged
merged 3 commits into from
Jun 27, 2023
Merged

Add tertiary phone number #102

merged 3 commits into from
Jun 27, 2023

Conversation

jensschuppe
Copy link
Collaborator

... just as #52 did for a secondary phone number, i. e. a phone3 parameter and configuration for the phone type.

@jensschuppe jensschuppe added feature status:needs review Code needs review and testing labels Jun 22, 2023
@jensschuppe jensschuppe added this to the 1.11 milestone Jun 22, 2023
@jensschuppe
Copy link
Collaborator Author

@jaapjansma fancy reviewing this, as you authored the secondary phone thing?

This could also use a test which I haven't had the time adding.

Copy link
Member

@bjendres bjendres left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I haven't tested.

I'm fairly sure it needs to be "tertiary" instead of "ternary", just so we don't confuse English native speakers. But if you don't want to change it, i'll add a "sic" :)

Let me know if you need/want me to actually test it.

@jensschuppe
Copy link
Collaborator Author

jensschuppe commented Jun 23, 2023

Let me know if you need/want me to actually test it.

@bjendres There's a unit test for secondary phone which might be easily adjusted oder copied for testing phone3, feel free to add one if you like.

@jensschuppe jensschuppe changed the title Add ternary phone number Add tertiary phone number Jun 23, 2023
@bjendres
Copy link
Member

Let me know if you need/want me to actually test it.

@bjendres There's a unit test for secondary phone which might be easily adjusted oder copied for testing phone3, feel free to add one if you like.

I did, but I cannot assess whether this copy tests all the right things. But the tests all pass now.

@jensschuppe jensschuppe merged commit 9fd9af6 into master Jun 27, 2023
@jensschuppe jensschuppe deleted the ternaryPhoneNumber branch June 27, 2023 10:31
@jensschuppe jensschuppe added status:fixed The issue has been resolved (usually by committing/merging code) and removed status:needs review Code needs review and testing labels Jun 27, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature status:fixed The issue has been resolved (usually by committing/merging code)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants