-
-
Notifications
You must be signed in to change notification settings - Fork 215
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: add mac address validation #270
feat: add mac address validation #270
Conversation
✅ Deploy Preview for valibot canceled.
|
d316be5
to
2594a13
Compare
Thank you for creating this PR! I plan to include this in our next release. |
@fabian-hiller thanks, valibot looks like nice lightweight validation library. Looking forward to contribute more to the project. |
Thank you so much! Your words mean a lot to me. I plan to reach v1 beta in January. In about 3 weeks I will be able to work full time on my open source projects again. |
Your implementation looks good. I'm just not sure what a valid MAC address is. Do all MAC addresses have to be 12 hexadecimal numbers long? Who defines this standard? Is there a clear source? The following formats are clear:
But I am still unsure about the following formats:
|
Also I am unsure if we should rename the function to |
There are 48 bit (IP4 style ) and 64 bit (IPv6 style ) MAC addresses. MAC addresses are formed according to the principles of two numbering spaces based on extended unique identifiers (EUIs) managed by the Institute of Electrical and Electronics Engineers (IEEE): EUI-48—which replaces the obsolete term MAC-48—and EUI-64. Regarding naming, i was allready thinking to name it macAddress, but since you have naming ipv4 and ipv6 instead ipV4Address i name it just mac, but i think macAddress, could be more meaningfull. Maybe we should split to 2 validators: mac48Address and mac64Address
|
I think both |
@fabian-hiller i believe current implementation should be ok, maybe we should just split to different validations, how it is done for ip, ipv4, ipv6 or leave it as it is for now. |
How would you divide it and also name it? |
Should we split it into |
@fabian-hiller i believe this would be a good idea, since mac64 supports newer 64 bit identifiers, which were added for IoT purpose. Will add this in the next days, so it could be merged in next release. |
Thank you for your contribution! I will review and merge this PR soon. |
5090c24
to
41bc49d
Compare
I'll merge this PR. Can you still confirm again that the following formats are valid? MAC 48:
MAC 64:
|
@fabian-hiller per my research this are valid formats for 48 and 64 bit MAC address. |
v0.23.0 is now available. |
No description provided.