-
-
Notifications
You must be signed in to change notification settings - Fork 332
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
ref: Deprecate not needed option sdkInfo
#1960
Conversation
SentryMeta is now the only source of truth. The values can be overridden by hybrid SDKs via SentrySDKOnly. Closes #1884
It really feels to me that we should deprecate the entire |
Why is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost LGTM. I'm looking into why testVersion
is failing at the moment.
@kevinrenskers, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if the tests will succeed. We could add resetSDKInfo, that is going to restore the default values, make it only visible for testing, and call it in tearDown in the ObjC tests and in clearTestState. For only making it visible for testing, we can create a test category like, for example, SentryClient+TestInit.h
Looks like all tests are passing now 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
sdkInfo
📜 Description
options.sdkInfo is deprecated; SentryMeta is now the only source of truth. The values can be overridden by hybrid SDKs via PrivateSentrySDKOnly.
💡 Motivation and Context
Closes #1884
💚 How did you test it?
All the existing tests still run after this refactor. Added new tests.
📝 Checklist
#skip-changelog
🔮 Next steps