Skip to content

Improved italian phone numbers #937

New issue

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

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

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

Catlike14
Copy link

What is the reason for this PR?

  • A new feature
  • Fixed an issue (resolve #ID)

Author's checklist

Summary of changes

Italian phone numbers start with 3 (for mobile) or 0 (for landline). They typically have 10 digits but landlines may also have 9 digits, excluding the +39 international prefix.

Review checklist

  • All checks have passed
  • Changes are added to the CHANGELOG.md
  • Changes are approved by maintainer

Italian phone numbers start with 3 (for mobile) or 0 (for landline). They typically have 10 digits but landlines may also have 9 digits, excluding the +39 international prefix.
@pimjansen
Copy link

PR not valid

@pimjansen pimjansen closed this Feb 20, 2025
@Catlike14
Copy link
Author

PR not valid

Can you clarify what exactly is invalid? If you're referring to the failed Roave BC Check test, it doesn’t seem related to my changes, as the errors appear to be about other issues

@pimjansen
Copy link

  • Changes are not documenten in changelog
  • There are no unittests

And the change is against 2.c instead of the 1.x branch

@Catlike14
Copy link
Author

To be honest, I'm not entirely sure which branch I should have used. I simply found an incorrect format and fixed it without worrying about the branch, also no unit test existed before my change. I'm happy to help write a unit test and open a new PR for 1.x when I have some free time.

@Catlike14 Catlike14 mentioned this pull request Feb 21, 2025
7 tasks
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.

2 participants