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: Add maxCacheItems #1017

Merged
merged 7 commits into from
Mar 24, 2021
Merged

feat: Add maxCacheItems #1017

merged 7 commits into from
Mar 24, 2021

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Mar 24, 2021

📜 Description

Add maxCacheItems to SentryOptions to make the number of maximum envelopes
to keep in the cache configurable.

💡 Motivation and Context

Fixes GH-899

💚 How did you test it?

Unit tests.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the CHANGELOG if needed
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

@philipphofmann philipphofmann requested a review from a team March 24, 2021 09:42
Copy link
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.

LGTM but I raised a discussion about the value. 30's been our standard so far but since it had in-memory storage in mind, maybe we reconsider?

@philipphofmann
Copy link
Member Author

philipphofmann commented Mar 24, 2021

LGTM but I raised a discussion about the value. 30's been our standard so far but since it had in-memory storage in mind, maybe we reconsider?

@bruno-garcia I'm also fine with increasing this, but it should be consistent across SDKs. Currently, we have 30 on Android and .NET.

@philipphofmann philipphofmann merged commit 0073a23 into master Mar 24, 2021
@philipphofmann philipphofmann deleted the feat/max-cache-items branch March 24, 2021 16:15
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.

Add maxCacheItems to SentryOptions
2 participants