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

Use built-in type definitions for uuid #1384

Merged
merged 1 commit into from
Dec 17, 2024
Merged

Use built-in type definitions for uuid #1384

merged 1 commit into from
Dec 17, 2024

Conversation

jasonaowen
Copy link
Contributor

Version 11 of the uuid library included a migration to TypeScript, so the externally-provided @types/uuid is no longer needed. Uninstall it.

We upgraded to version 11 in PR #1285.

@jasonaowen jasonaowen added the dependencies Pull requests that update a dependency file label Dec 16, 2024
@jasonaowen jasonaowen requested review from slifty and bickelj December 16, 2024 22:58
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.40%. Comparing base (00fe59c) to head (25816ec).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1384   +/-   ##
=======================================
  Coverage   87.40%   87.40%           
=======================================
  Files         186      186           
  Lines        2413     2413           
  Branches      325      318    -7     
=======================================
  Hits         2109     2109           
- Misses        278      304   +26     
+ Partials       26        0   -26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@bickelj bickelj left a comment

Choose a reason for hiding this comment

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

It is always nice to see unneeded code removed. I did not try it locally but the tests, IIRC, include some use of Uuid, so since all that passed I think this is going to be OK.

@@ -10320,6 +10313,7 @@
"https://github.com/sponsors/broofa",
"https://github.com/sponsors/ctavan"
],
"license": "MIT",
Copy link
Contributor

Choose a reason for hiding this comment

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

Strange that this line was added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that too; I think I updated npm, but I'm not sure what version from, and I can't find anything obvious in the npm changelog. The upstream package.json license hasn't changed in almost a decade.

Version 11 of the uuid library[1] included a migration to TypeScript, so
the externally-provided `@types/uuid` is no longer needed. Uninstall it.

[1] https://github.com/uuidjs/uuid/blob/main/CHANGELOG.md#1100-2024-10-26
@jasonaowen jasonaowen merged commit d08dd8d into main Dec 17, 2024
11 checks passed
@jasonaowen jasonaowen deleted the remove-uuid-types branch December 17, 2024 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants