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

feature(IE Postal): add postal code for Ireland #1083

Merged
merged 1 commit into from
Aug 12, 2019

Conversation

ezkemboi
Copy link
Member

@@ -28,6 +28,7 @@ const patterns = {
HR: /^([1-5]\d{4}$)/,
HU: fourDigit,
ID: fiveDigit,
IE: /^[a-zA-Z0-9]{3}\b \b[a-zA-Z0-9]{4}$/i,
Copy link
Member

@profnandaa profnandaa Aug 12, 2019

Choose a reason for hiding this comment

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

Simplified regex would be:

/^\w{3}\s\w{4}$/i

\w will match any alphanumeric, see a good quick ref here.

However, it also looks to me like most of the areas, the routing key is in the format of [alpha][digit][digit] except for Dublin 6W, so we could make it more specific?

/^[A-z]\d[\d|W]\s\w{4}$/i

nb: the i can be left out too since the regex is wide enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

Checking on this @profnandaa.

Copy link
Member Author

Choose a reason for hiding this comment

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

@profnandaa, it should be

/^[A-z]\d[\d|w]\s\w{4}$/i

Instead of

/^[A-z]\d[\d|W]\s\w{4}$/i

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, \s will match tab and new line,
Should we allow for tabs and a new line for separation?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, you we could just do an actual space.

Copy link
Member

Choose a reason for hiding this comment

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

/^[A-z]\d[\d|w] \w{4}$/

@profnandaa profnandaa added the 🧹 needs-update For PRs that need to be updated before landing label Aug 12, 2019
- add postal code for ireland as requested by @JoeEarly from issue validatorjs#784
- fixes request done on issue validatorjs#784
@ezkemboi
Copy link
Member Author

@profnandaa updates made as requested.

Copy link
Member

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

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

LGTM

@profnandaa
Copy link
Member

And as always, thanks for the PR, Ez! 🎉

@profnandaa profnandaa merged commit 35a72b6 into validatorjs:master Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 needs-update For PRs that need to be updated before landing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants