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

Feat: Introduce Hint data bag #1136

Merged
merged 44 commits into from
Dec 12, 2022
Merged

Feat: Introduce Hint data bag #1136

merged 44 commits into from
Dec 12, 2022

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Nov 15, 2022

📜 Description

Introduces a custom type for all hint properties, with a Map like interface.

Relates to #510

💡 Motivation and Context

Users can attach more than one primitive or object to API which takes a hint parameter.

This is a breaking change.

💚 How did you test it?

Added unit tests.

📝 Checklist

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

🔮 Next steps

I have opted not to add Attachments or Screenshot to the scope of this PR. Should we do this separately and also define what to do with this data in the SDK?

@github-actions
Copy link
Contributor

github-actions bot commented Nov 15, 2022

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Introduce `Hint` data bag ([#1136](https://github.com/getsentry/sentry-dart/pull/1136))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against bfe155c

@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2022

Codecov Report

❗ No coverage uploaded for pull request base (v7.0.0@5460121). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##             v7.0.0    #1136   +/-   ##
=========================================
  Coverage          ?   88.07%           
=========================================
  Files             ?      121           
  Lines             ?     3815           
  Branches          ?        0           
=========================================
  Hits              ?     3360           
  Misses            ?      455           
  Partials          ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@denrase denrase marked this pull request as ready for review November 15, 2022 14:36
dart/lib/src/hint.dart Outdated Show resolved Hide resolved
@marandaneto
Copy link
Contributor

@denrase so one thing that has to be done is, the SDK should create a Hint if none is provided, for example, Java does it on SentryClient#captureEvent.
Also, we should always pass the original object as a hint when automatically capturing events, for example.
LoggingIntegration#_onLog calls captureEvent and addBreadcrumb not passing the record as a hint, we should do that for all the integrations that call captureEvent, captureException, etc...

@marandaneto
Copy link
Contributor

Btw before merging this, I believe we need to update the branch v7.0.0 with the latest main to avoid more conflicts.

@denrase
Copy link
Collaborator Author

denrase commented Nov 21, 2022

@marandaneto Hope i found all occurences where we can pass a hint. Made an expeption (pun!) to exceptions and stacktraces, as they are already passed. Let me know what you think.

@denrase denrase requested a review from marandaneto December 6, 2022 10:58
Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

Left 2 comments but overall LGTM

@denrase denrase merged commit 3e6109b into v7.0.0 Dec 12, 2022
@denrase denrase deleted the feat/introduce-hint-data-bag branch December 12, 2022 12:33
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.

3 participants