Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: allow setting fingerprint from Sentry module #1367

Closed
wants to merge 1 commit into from
Closed

feat: allow setting fingerprint from Sentry module #1367

wants to merge 1 commit into from

Conversation

fsateler
Copy link
Contributor

This way configure_scope is not needed.

Thanks for your Pull Request 🎉

Please keep these instructions in mind so we can review it more efficiently:

  • Add the references of all the related issues/PRs in the description
  • Whether it's a new feature or a bug fix, make sure they're covered by new test cases
  • If this PR contains any refactoring work, please give it its own commit(s)
  • Finally, please add an entry to the corresponding changelog

Other Notes

  • We squash all commits before merging
  • We generally review new PRs within a week
  • If you have any question, you can ask for feedback in our discord community first

Description

Describe your changes:

This way `configure_scope` is not needed.
@st0012
Copy link
Collaborator

st0012 commented Apr 29, 2021

@rhcarvalho I don't see a reason to reject this. do other SDKs support the top-level set_fingerprint as well?

@rhcarvalho
Copy link
Contributor

The SDK team has discussed this yesterday.

Other SDKs don't support this shortcut. Before we add more global methods there needs to be a broader discussion about where the API is heading, as we want to avoid making one-off decisions.

The set_fingerprint method, in particular, is meant to change the fingerprint of a single event, and thus having this global shortcut could bring unintended side-effects and usage error.

The decision was to not merge this now, collect the data point, and defer the broader discussion for later.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants