Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: remove uuid dependency in favor of crypto package #443

Closed
wants to merge 3 commits into from

Conversation

lance
Copy link
Member

@lance lance commented Nov 19, 2021

Proposed Changes

As of Node.js 14.17.0 the crypto package has a randomUUID() function
which we can use to replace our external dependency.

Signed-off-by: Lance Ball lball@redhat.com

As of Node.js 14.17.0 the crypto package has a `randomUUID()` function
which we can use to replace our external dependency.

Signed-off-by: Lance Ball <lball@redhat.com>
@lance lance added the chore/refactor Refactoring and other non-functional changes to module code label Nov 19, 2021
@lance lance requested a review from a team November 19, 2021 13:43
@lance lance self-assigned this Nov 19, 2021
Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Lance Ball <lball@redhat.com>
@lance
Copy link
Member Author

lance commented Nov 19, 2021

Note that this change removes support for Node.js 12.x which is not yet EOL. We may want to consider holding off on this until April 2022 when that happens.

@matthewrobertson
Copy link

I would probably wait a while after Node.js 12 goes EOL before you land this so developer have time to migrate.

Copy link
Member

@grant grant left a comment

Choose a reason for hiding this comment

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

As mentioned, I don't recommend removing support for Maintenance LTS versions of Node. This is a good reference, but we shouldn't remove this version from support or tests.

For example, many customers are still running Node 10 applications and haven't upgraded their Node version.

@lance
Copy link
Member Author

lance commented Nov 19, 2021

As mentioned, I don't recommend removing support for Maintenance LTS versions of Node. This is a good reference, but we shouldn't remove this version from support or tests.

For example, many customers are still running Node 10 applications and haven't upgraded their Node version.

Yikes! That's been EOL for a while. In any case, y'all are probably right. I was just eager to show that we don't like to carry a lot of dependencies and a willingness to remove them. :)

@lance lance closed this Nov 19, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
chore/refactor Refactoring and other non-functional changes to module code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants