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

Device context for transactions #1381

Closed
wants to merge 2 commits into from

Conversation

marandaneto
Copy link
Contributor

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

Comment on lines 6 to 10
// other approach is to use EventProcessor<T extends SentryBaseEvent>
// and having a processor which is the common fields and specific processors for Event and Transactions
// not sure if our event processor interface should be deal with this
// the downside using only the base class, if people want to process a field which is not part of SentryBaseEvent
// they need to cast it manually
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bruno-garcia can we discuss this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after the discussion, we could try to use default methods for interfaces, return it so it's not a breaking change.

@marandaneto
Copy link
Contributor Author

favor of #1430

@marandaneto marandaneto deleted the feat/transaction-device-context branch May 4, 2021 08:16
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.

1 participant