Skip to content

Conversation

@WikiRik
Copy link
Member

@WikiRik WikiRik commented Nov 10, 2021

This PR adds support for validating against EUI-64 as well as the previous EUI-48. This corresponds with the macaddr8 type in postgres.

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)

@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #1865 (5c398a4) into master (47ee5ad) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1865   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          102       102           
  Lines         2059      2072   +13     
  Branches       464       472    +8     
=========================================
+ Hits          2059      2072   +13     
Impacted Files Coverage Δ
src/lib/isMACAddress.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47ee5ad...5c398a4. Read the comment docs.

Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

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

Thank you for your PR @WikiRik !
Can you please address my comments?
I'm also wondering if EUI-48/64 is the most used technical term.Personally, I know it as MacAddr6 and MacAddr8.

@tux-tn tux-tn added 🧹 needs-update For PRs that need to be updated before landing 🎉 first-pr labels Nov 12, 2021
@WikiRik
Copy link
Member Author

WikiRik commented Nov 12, 2021

To be honest, I do not really know. I would be happy to refer to it as MacAddr6 and 8. Do you think I should refactor it to numberOfBits? Or some other suggestion?

@tux-tn
Copy link
Member

tux-tn commented Nov 15, 2021

@WikiRik Looks like IEEE and IETF are using EUI-48 and EUI-64 terminology (RFC7043 as an example). I suggest we stick to that, we can add aliases later if community is complaining or suggesting another name.

@WikiRik WikiRik requested a review from tux-tn November 15, 2021 22:37
Copy link
Member

@tux-tn tux-tn left a 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 your efforts @WikiRik !

@tux-tn tux-tn added ready-to-land For PRs that are reviewed and ready to be landed and removed 🧹 needs-update For PRs that need to be updated before landing labels Nov 15, 2021
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, thanks for your contrib! 🎉

@profnandaa profnandaa merged commit f055c11 into validatorjs:master Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎉 first-pr ready-to-land For PRs that are reviewed and ready to be landed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants