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.

Sign up for GitHub

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

Jump to bottom

refactor(impl/domain): use native uuid generator #103

Merged
merged 3 commits into from Jan 18, 2022
Merged

refactor(impl/domain): use native uuid generator #103

merged 3 commits into from Jan 18, 2022

Conversation

twlite
Copy link
Contributor

@twlite twlite commented Jan 16, 2022

@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2022 •
edited

Codecov Report

Merging #103 (23686cb) into main (da2b4e6) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 23686cb differs from pull request most recent head 3835689. Consider uploading reports for the commit 3835689 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #103   +/-   ##
=======================================
  Coverage   87.83%   87.83%           
=======================================
  Files           5        5           
  Lines         148      148           
  Branches       25       25           
=======================================
  Hits          130      130           
  Misses         15       15           
  Partials        3        3
Impacted Files Coverage Δ
src/impl/domain.ts 97.43% <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 da2b4e6...3835689. Read the comment docs.

@mesaugat mesaugat self-requested a review January 18, 2022 12:45
@mesaugat mesaugat added the dependencies Pull requests that update a dependency file label Jan 18, 2022
mesaugat
mesaugat approved these changes Jan 18, 2022
View reviewed changes
Copy link
Member

@mesaugat mesaugat left a comment •
edited

Choose a reason for hiding this comment

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

LGTM. Merging this change will drop support for Node 14.17.0 and below. Thus, this is a major change. We will have to bump the package version to v2.0.0.

@mesaugat mesaugat merged commit 0b51c52 into leapfrogtechnology:main Jan 18, 2022
@twlite twlite deleted the refactor/uuid branch January 18, 2022 13:17
mesaugat added a commit that referenced this pull request Jan 20, 2022
@mesaugat
Set node engine version to 14.17.0 and above
431d954 
Started using NodeJS crypto module to generate random UUIDs instead of
using the uuid package.
@see #103

This means support for Node 12 has been dropped as
`crypto.randomUUID([options])` works only on node v14.17.0 and above.
@see https://nodejs.org/api/crypto.html#cryptorandomuuidoptions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mesaugat mesaugat mesaugat approved these changes

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@twlite @codecov-commenter @mesaugat