-
-
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
fix(isMacAddress): improve regexes and options #1616
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1616 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 100 100
Lines 1796 1794 -2
=========================================
- Hits 1796 1794 -2
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.
Thank you @fedeci for your quick fix on this.
I can see that the issue here is related to the macAddressWithDots
regex. The author forgot to escape the dot. I agree with your fix of the regex but I don't understand why you decided to change the behaviour (Hyphens and spaces are considered valid MAC Address separators) and the option name Introducing potential breaking changes (I know that no_colons
is still supported in your code).
I thought it was possible to join multiple suggested changes in one commit🤦 |
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.
Thank your @fedeci for making the changes that fast 🎉 LGTM
Closing and re-opening to kick off Github actions. |
Fixes #1614
This PR simplifies regexes, update the
no_colons
option name tonoSeparators
. FixesmacAddressWithDots
regex to only allow dots as separator.Checklist