Skip to content

Conversation

@mydea
Copy link
Member

@mydea mydea commented Jan 5, 2024

This finally deprecates startTransaction(). There are only few remaining usages in our own code base, which we can refactor as we go.

I chose to leave usages in E2E/integration tests for now, we can then refactor them when we get rid of these to ensure the behavior remains the same.

@mydea mydea requested review from AbhiPrasad, Lms24 and lforst January 5, 2024 09:08
@mydea mydea self-assigned this Jan 5, 2024
@mydea mydea force-pushed the fn/deprecate-startTransaction branch from f795461 to 6cb3c2b Compare January 5, 2024 09:12
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Nice!

Comment on lines +159 to +162
return parentSpan
? parentSpan.startChild(ctx)
: // eslint-disable-next-line deprecation/deprecation
hub.startTransaction(ctx);
Copy link
Member

Choose a reason for hiding this comment

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

not relevant for this PR but this makes me realize we'll probably need to create an internal startTxn utility for us as a replacement of hub.startTransaction when its removed as long as we're still sending spans in transacitons.

Copy link
Member Author

Choose a reason for hiding this comment

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

jup, also had that thought - for now we'll still need some internal thing, but we can refactor this on the v8 branch then I'd say :)

@mydea mydea merged commit 89ca41d into develop Jan 5, 2024
@mydea mydea deleted the fn/deprecate-startTransaction branch January 5, 2024 10:17
c298lee pushed a commit that referenced this pull request Jan 9, 2024
This finally deprecates `startTransaction()`. There are only few
remaining usages in our own code base, which we can refactor as we go.

I chose to leave usages in E2E/integration tests for now, we can then
refactor them when we get rid of these to ensure the behavior remains
the same.

---------

Co-authored-by: Lukas Stracke <[email protected]>
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.

4 participants