Skip to content
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

Bugfix: Encoding of country code in FormatOutOfCountryCallingNumber #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jojomi
Copy link

@jojomi jojomi commented May 30, 2017

Minimal testcase:

phonenumber, err := libphonenumber.Parse("+3587884512974", "DE")
fmt.Println(libphonenumber.FormatOutOfCountryCallingNumber(phonenumber,
"DE"))

This change is Reviewable

…Number

Minimal testcase:

```
phonenumber, err := libphonenumber.Parse("+3587884512974", "DE")
fmt.Println(libphonenumber.FormatOutOfCountryCallingNumber(phonenumber,
"DE"))
```
@ttacon
Copy link
Owner

ttacon commented Jun 2, 2017

Hi! Thanks for the PR! A few questions:

  1. Can you add a test to the test code to show what you're trying to fix here as well (I know that it's in the comment above but if it's a test case we're fixing it should also go into the unit tests)?

  2. What exactly is the issue that you're trying to resolve? I'd prefer not to incur multiple allocations to do an integer -> []byte conversion.

Thanks!

@jojomi
Copy link
Author

jojomi commented Jun 4, 2017

Thank you for having a look.

  1. I can try, but currently my time is limited. I am okay with leaving this PR open until added
  2. Is there a quicker way to get from an int to []byte? The point is that the bit wizardry seems wrong there, the country code integer just needs to be printed. The current code yields seemingly random (non-numerical) characters depending on what character codes are mapped after shifting around.

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.

3 participants