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

Add options for MAC address validation without colons, and Numeric validation without symbols #847

Closed
wants to merge 3 commits into from

Conversation

Maxattax97
Copy link
Contributor

Includes tests and updated README.md.

@Maxattax97 Maxattax97 changed the title Add MAC address validation without colons Add options for MAC address validation without colons, and Numeric validation without symbols Jun 13, 2018
@profnandaa
Copy link
Member

Max, thanks for your contribution!
(1) I would like to understand the essence of checking MAC addresses without the colons? Are there cases where MAC addresses don't have colons?
(2) Would be nice to split this PR into two, one for the MAC address issue, and other for the isNumeric

-na

@Maxattax97
Copy link
Contributor Author

In the real world, no, MAC addresses should be in a format with colons. However, there are many cases when you are dealing with MAC addresses frequently on forms and such where it's much more portable, easier to type, and (arguably) more human-readable to work without colons.

In my use case, I have a database full of MAC addresses without colons as well as many internal services that I call into which also use this format. I thought other people may have a similar situation, so I accordingly made an option but kept the default functionality.

Splitting makes since, I will take care of that. I apologize, I'm a bit new to submitting PR's, so I wasn't aware that it would take all commits from my forked repo up until the PR was accepted.

@profnandaa
Copy link
Member

profnandaa commented Jun 15, 2018

Splitting makes since, I will take care of that. I apologize, I'm a bit new to submitting PR's, so I wasn't aware that it would take all commits from my forked repo up until the PR was accepted.

The best way is to create a branch for each feature, and not committing from your fork's master.

The rest makes sense to me, thanks for explaining 👍

@Maxattax97
Copy link
Contributor Author

That was messy, but it was small and simple enough to learn a couple new git commands.

I'll close this so that the new PR's may address the features individually.

@Maxattax97 Maxattax97 closed this Jun 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants