-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat(isLicensePlate): add support for Swedish license plates #1665
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1665 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 102 102
Lines 2072 2073 +1
Branches 472 472
=========================================
+ Hits 2072 2073 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please fix the merge conflict and we should be good to go.
@profnandaa Fixed the merge conflict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @elmaxe and thank you for your PR. Can you please check my comments below?
In addition to that i have two more general feedbacks:
- Multiple white-spaces are considered a valid license plate. You should probably trim your string before passing it to the regex validation in order to prevent that.
- According to the wikipedia page of swedish license plates, a plate can't end with the
O
character to prevent it from being confused with0
, i don't see that exception in your regex, am i wrong?
Fair!
Interestingly this is not on the Swedish wikipedia page, but it is correct (source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉 Thank you for taking the time to address my comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! 🎉
This PR adds support for Swedish license plates.
Checklist