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

Replace shortid to nanoid #1866

Merged
merged 1 commit into from
Nov 29, 2017
Merged

Replace shortid to nanoid #1866

merged 1 commit into from
Nov 29, 2017

Conversation

ai
Copy link
Contributor

@ai ai commented Oct 10, 2017

What do you think of replacing shortid to nanoid (without big changes for users). Benefits:

  1. shortid is not maintained anymore (and issue tracker has many important issues). I tried to took maintenance, but found that I can’t fix shortid with keeping API.
  2. shortid has problem with random generator (issue) and ID uniformity (issue). nanoid use hardware random generator and has special uniformity tests (docs).
  3. shortid has the problem with - symbol in the end of URL on some Android app (issue). nanoid uses safe ~ instead of -.
  4. nanoid is 10x times faster than shortid.

@AlexanderMoskovkin @AndreyBelym what do you think?

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 2ccdf22 have failed. See details:

1 similar comment
@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 2ccdf22 have failed. See details:

@AndreyBelym
Copy link
Contributor

Hello @ai !

Thank you, nice job! I'll figure out why tests are failing, likely it's something with Firefox on Browserstack. So your PR will be merged when I resolve issues with tests.

One more point: we are using shortid in testcafe-hammerhead (https://github.com/DevExpress/testcafe-hammerhead) too. Are you going to replace it with nanoid there? Otherwise I'll do it by myself.

@ai
Copy link
Contributor Author

ai commented Oct 11, 2017

@AndreyBelym done DevExpress/testcafe-hammerhead#1343

@AndreyBelym
Copy link
Contributor

You are awesome!

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 2ccdf22 have failed. See details:

@testcafe-build-bot
Copy link
Collaborator

✅ Tests for the commit 71491cd have passed. See details:

@AlexanderMoskovkin AlexanderMoskovkin merged commit deeb7ae into DevExpress:master Nov 29, 2017
kirovboris pushed a commit to kirovboris/testcafe-phoenix that referenced this pull request Dec 18, 2019
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.

4 participants