Skip to content

Hints are now used via a Hints object and passed into beforeSend and EventProcessor as @NotNull Hints object - #2045

Merged
adinauer merged 10 commits into
6.x.xfrom
feat/replace-map-with-hints-object
May 23, 2022
Merged

Hints are now used via a Hints object and passed into beforeSend and EventProcessor as @NotNull Hints object#2045
adinauer merged 10 commits into
6.x.xfrom
feat/replace-map-with-hints-object

Conversation

@adinauer

@adinaueradinauer commented May 13, 2022

Copy link
Copy Markdown
Member

📜 Description

Replaces Map<String, Object> for hints with a Hints object. Also changed from @Nullable to @NotNull in beforeSend and EventProcessor.

💡 Motivation and Context

This makes it easier for developers to interact with hints without having to deal with certain cases (e.g. hints == null, hints not having an attachment entry, hints having a null attachment entry, ...)

See getsentry/sentry-javascript#5036

Here's another PR building on top of this to allow manipulation of attachments via beforeSend and eventProcessor: TODO

💚 How did you test it?

Unit Tests, see linked PR

📝 Checklist

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

🔮 Next steps

  • Update docs

@adinaueradinauer changed the title Replace hint Map with Hints objectHints are now used via a Hints object and passed into beforeSend and EventProcessor as @NotNull Hints objectMay 13, 2022
@codecov-commenter

codecov-commenter commented May 13, 2022

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (6.x.x@191ca19). Click here to learn what that means.
The diff coverage is n/a.

@@ Coverage Diff @@## 6.x.x #2045 +/- ##
========================================
Coverage ? 81.06% Complexity ? 3211 ========================================
Files ? 229 Lines ? 11804 Branches ? 1568 ========================================
Hits ? 9569 Misses ? 1650 Partials ? 585 

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 191ca19...a7b0796. Read the comment docs.

@philipphofmannphilipphofmann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for doing this. I only gave it a high level API review.

Comment threadsentry/src/main/java/io/sentry/hints/Hints.java Outdated
Comment threadsentry/src/main/java/io/sentry/hints/Hints.java Outdated
Comment threadsentry/src/main/java/io/sentry/util/LogUtils.java Outdated
adinauerand others added 3 commits May 20, 2022 07:32
* Store attachments in hints and allow manipulation in beforeSend and eventProcessor
* Add changelog
* Add tests for breadcrumbs and attachments via hints
* Update CHANGELOG.md
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
* Rename AttachmentContainer to Attachments
* Use long for test
* Move attachments into Hints
* Fix kotlin/java interop for Hints
* Convert screenshot from map entry to property
* Rename hint name param
* Rename clear to clearAttachments
* Use kotlin short version access for getScreenshot
* Move AttachmentsTest into HintsTest; add param names
* Make primitiveMapping table static
* Use ArrayList as there should not be a synchronization issue for hints
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
Comment threadsentry/src/main/java/io/sentry/OutboxSender.java
Comment threadsentry/src/main/java/io/sentry/TypeCheckHint.java Outdated
Comment threadsentry/src/main/java/io/sentry/OutboxSender.java
Comment threadsentry/src/test/java/io/sentry/SentryClientTest.kt Outdated
Comment threadsentry/src/test/java/io/sentry/SentryClientTest.kt Outdated
Comment threadsentry/src/test/java/io/sentry/SentryClientTest.kt Outdated
@adinauer
adinauer merged commit 134eb16 into 6.x.xMay 23, 2022
@adinauer
adinauer deleted the feat/replace-map-with-hints-object branch May 23, 2022 08:03
Sign up for freeto 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.

5 participants

@adinauer@codecov-commenter@philipphofmann@romtsn@marandaneto