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: Support setting maxCacheItems #2102

Merged
merged 4 commits into from
Mar 4, 2022
Merged

feat: Support setting maxCacheItems #2102

merged 4 commits into from
Mar 4, 2022

Conversation

marandaneto
Copy link
Contributor

@marandaneto marandaneto commented Mar 3, 2022

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

feat: Support setting maxCacheItems

💡 Motivation and Context

#1452

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

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.

We need to find out way to test stuff here

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 1f6de32

@marandaneto
Copy link
Contributor Author

We need to find out way to test stuff here

Yep, #1874
Just need to prioritize that.

@bruno-garcia
Copy link
Member

We don't need to add unit tests. In fact, I believe ideally we would test this with integration or e2e tests.
e2e already exist

@marandaneto
Copy link
Contributor Author

We don't need to add unit tests. In fact, I believe ideally we would test this with integration or e2e tests. e2e already exist

This is just passing custom configuration to the Native SDKs, if you want to test the "maxCacheItems" feature, the Native SDKs already do it.
IMO, this type of change, only a unit test would be fine.
@philipphofmann opinions? Since we both discussed about #1874

@marandaneto marandaneto merged commit 3f0c170 into main Mar 4, 2022
@marandaneto marandaneto deleted the feat/maxCacheItems branch March 4, 2022 16:58
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.

2 participants