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

fix(instrumentation-ioredis): make @types/ioredis a devDependency #2069

Merged

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented Mar 29, 2024

Which problem is this PR solving?

  • @types/ioredis is listed as a dependency even though it's types are not part of the `@opentelemetry/instrumentation-ioredis package's public API

Short description of the changes

  • move @types/ioredis to devDependencies to avoid it being installed for consumers of the package.

Testing

Tested locally by running npm pack and then installing the package into a local package with typescript and without @types/ioredis installed. Imported and instantiated the instrumentation to check that everything still compiles correctly.

Fixes #2059

Copy link

codecov bot commented Mar 29, 2024

Codecov Report

Merging #2069 (be847fd) into main (dfb2dff) will decrease coverage by 0.16%.
Report is 46 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2069      +/-   ##
==========================================
- Coverage   90.97%   90.82%   -0.16%     
==========================================
  Files         146      148       +2     
  Lines        7492     7672     +180     
  Branches     1502     1537      +35     
==========================================
+ Hits         6816     6968     +152     
- Misses        676      704      +28     

see 8 files with indirect coverage changes

@pichlermarc pichlermarc marked this pull request as ready for review March 29, 2024 14:44
@pichlermarc pichlermarc requested a review from a team March 29, 2024 14:44
@trentm trentm enabled auto-merge (squash) April 9, 2024 22:51
@trentm trentm disabled auto-merge April 9, 2024 22:51
@trentm
Copy link
Contributor

trentm commented Apr 9, 2024

^^ @pichlermarc I enabled, and then disabled auto-merge. This is the first time I tried auto-merge on a PR from a repo that doesn't allow commits to the feature branch from the PR, so I cannot update with the base branch.

@pichlermarc
Copy link
Member Author

@trentm this must've happened as I'm using an organization fork and did not have maintainer edits enabled.

I run a script to enable edits on my PRs via the API as GitHub does not allow enabling maintainer edits via the UI.
I forgot to run the script for this PR (turned it on for all my PRs just now).

@pichlermarc pichlermarc merged commit a172f8a into open-telemetry:main Apr 10, 2024
17 checks passed
@pichlermarc pichlermarc deleted the fix/ioredis-types branch April 10, 2024 10:45
@dyladan dyladan mentioned this pull request Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@types/ioredis4 marked as dependency in @opentelemetry/instrumentation-ioredis
6 participants