Skip to content

Attachments can be manipulated via hints - Extra for attachment methods directly on Hints#2049

Closed
adinauer wants to merge 1 commit into
feat/allow-attachment-manipulation-via-hintsfrom
feat/allow-attachment-manipulation-via-hints-extra1
Closed

Attachments can be manipulated via hints - Extra for attachment methods directly on Hints#2049
adinauer wants to merge 1 commit into
feat/allow-attachment-manipulation-via-hintsfrom
feat/allow-attachment-manipulation-via-hints-extra1

Conversation

@adinauer
Copy link
Copy Markdown
Member

📜 Description

Extra for #2046 with attachment methods directly in Hints instead of a separate Attachments object.

💡 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

@github-actions
Copy link
Copy Markdown
Contributor

Fails
🚫 Please consider adding a changelog entry for the next release.

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

- Attachments can be manipulated via hints - Extra for attachment methods directly on Hints ([#2049](https://github.com/getsentry/sentry-java/pull/2049))

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 90bfee5

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (feat/allow-attachment-manipulation-via-hints@04edda0). Click here to learn what that means.
The diff coverage is n/a.

@@                               Coverage Diff                               @@
##             feat/allow-attachment-manipulation-via-hints    #2049   +/-   ##
===============================================================================
  Coverage                                                ?   80.96%           
  Complexity                                              ?     3177           
===============================================================================
  Files                                                   ?      229           
  Lines                                                   ?    11707           
  Branches                                                ?     1556           
===============================================================================
  Hits                                                    ?     9479           
  Misses                                                  ?     1646           
  Partials                                                ?      582           

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 04edda0...90bfee5. Read the comment docs.

Copy link
Copy Markdown
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

having a List<> instead of relying on the Map<String, Object> is ideal.
Other than that, LGTM

}

public void clearAttachments() {
internalStorage.put(SENTRY_ATTACHMENTS, new CopyOnWriteArrayList<Attachment>());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this could be a list instead

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will update

@adinauer
Copy link
Copy Markdown
Member Author

Decided to go with list property in Hints instead.

@adinauer adinauer closed this May 17, 2022
@romtsn romtsn deleted the feat/allow-attachment-manipulation-via-hints-extra1 branch July 19, 2023 09:55
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