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

Russia, Kazakhstan, Abhasia and South Osetia #49

Merged
merged 1 commit into from
Jul 2, 2012
Merged

Conversation

glebm
Copy link
Contributor

@glebm glebm commented Jul 1, 2012

This needs very careful review, as I don't quite understand some things in the DSL, and how the correct splitting gets chosen

Some NDCs with X subscriber digits are prefixes of other NDCs with Y subscriber digits (where X != Y), which might mean this does not work properly?

Best would be to talk to me on Skype so we can work out if this does the right thing or not...

floere added a commit that referenced this pull request Jul 2, 2012
	Russia, Kazakhstan, Abhasia and South Osetia
@floere floere merged commit 20ed1fc into floere:master Jul 2, 2012
@floere
Copy link
Owner

floere commented Jul 2, 2012

Hi Gleb,

First of all: Many thanks! Great job :)
Also, I'm afraid I was already asleep when the "Skype request" came in, since I currently live in Australia.
You are quite right regarding the prefixes. Phony will go through the rules from the front, so the rest of a number with NDC 84025 will be split in groups of 1, 2, 2. But an Abhas number which has not been matched before will be split 2, 2, 2. I will put two cases in the specs for you to look at.

Let's not worry about this too much. Phony is best effort, as perfection is simply not possible. A number will be displayed. It might not be perfect, but it will be displayed.

Cheers,
Florian

Thanks again!

@floere
Copy link
Owner

floere commented Jul 2, 2012

@floere
Copy link
Owner

floere commented Jul 2, 2012

Released in 1.7.6. Thanks again, @glebm !

@floere
Copy link
Owner

floere commented Jul 2, 2012

P.S: Note that I added 2 cases regarding the Abhasia (2-2-2) / Russia (1-2-2) distinction.

@floere
Copy link
Owner

floere commented Jul 5, 2012

Hi @glebm,

You were very right about giving it a careful review ;)
I read it thoroughly but failed to see one problem with it. Just FYI, if you use regexps for NDC detection, it needs a group in it so it knows what part NDC is (I can't take it all since some NDCs are dependent on what follows after – no kidding, it's stupid, but that's how phone numbers "work").
But it was my bad, this just as an information. Perhaps I am able to detect if there's a group in this rule and if not, it will fail early.

Anyway, many thanks again – if you feel the itch again, just send phony more pull requests! :)

Cheers,
Florian

@floere
Copy link
Owner

floere commented Jul 5, 2012

Ok, regex group check is implemented.

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