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

deps(opentelemetry-instrumentation): Bump shimmer types to 1.2.0 #4854

Closed

Conversation

lforst
Copy link
Contributor

@lforst lforst commented Jul 8, 2024

Which problem is this PR solving?

Bumps the shimmer package to a version that doesn't include an invasive type that breaks certain setups.

See PR for more details: DefinitelyTyped/DefinitelyTyped#69966

Fixes getsentry/sentry-javascript#12682

This PR supersedes #4835

Short description of the changes

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

I don't think tests are necessary here. Feel free to let me know though!

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

Copy link
Contributor

@trentm trentm left a comment

Choose a reason for hiding this comment

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

I'll probably leave this to @pichlermarc to review, as he groks the TS requirements better. (However, he's on vacation for a while now, so I'm not sure if that delay causes a lot of hardship.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think your PR accidentally re-added some git submodules that were removed a while back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct 😂 I haven't felt this stupid in a long time, but how the hell do I get rid of those again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Voodoo? Git submodules are ... a pain. I have to go Googling everytime they come up.

I think this is the SO question I always land on: https://stackoverflow.com/questions/1260748/how-do-i-remove-a-submodule
I recall using the steps in what is now the "Older community wiki instructions:" section of the top answer there.

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 just rm -rfed the repo and opened a new PR. Sorry about the inconvenience: #4865

@lforst lforst force-pushed the lforst-bump-shimmer-typedef branch 2 times, most recently from a141888 to cf7748f Compare July 10, 2024 07:22
@lforst
Copy link
Contributor Author

lforst commented Jul 15, 2024

Closed in favor of: #4865

@lforst lforst closed this Jul 15, 2024
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.

Sentry sdk adds "__wrapped"-properties to types, which prevents nestjs project from building
2 participants