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

Limit validation to hexadecimal values #20

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

click2install
Copy link

Currently, validation will accept zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz as a valid Guid. The changes in this pull request limit validation to hexadecimal characters only.

Code and test cases are updated to reflect this change.

@gekkedev
Copy link
Contributor

gekkedev commented Mar 7, 2021

I'm afraid the repo is not actively maintained anymore for over a year, so your change has a low chance to be merged. There are a few forks and as maintainer of one of them I'd be happy to accept your PR there with the following changes:

  1. updating dependencies - not recommended within the same PR (and in the fork already done)
    • you could remove it from this PR as well btw to increase the chances of getting it accepted
  2. Increment the version in package.json and package-lock.json.
  3. Due to my limited RegEx understanding I'm not sure if the few hardcoded GUIDs in the tests are enough. There is a test for generating 3000 GUIDs and validating them:
    https://github.com/NicolasDeveloper/guid-typescript/blob/2c4569d92bb60987bac9fef1d14e341654d8bba8/tests/guid.spec.ts#L63
    But: The checks in the other direction are only a few examples. What if, we create a JSON file of GUIDs (generated elsewhere) and import it in the tests to check for their validity?
    Just noticed it is not a good test: Unique GUID generation test is faulty ez-libs/ez-guid#42 - feel free to fix in the same PR

@snico-dev
Copy link
Owner

Sorry, everyone! I haven't been maintaining this repo for the past few years due to personal reasons. I'll review the requests and accept them soon. Thanks!

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.

3 participants