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

Undo Options initWithDict removal #2547

Closed
philipphofmann opened this issue Dec 20, 2022 · 3 comments · Fixed by #2579
Closed

Undo Options initWithDict removal #2547

philipphofmann opened this issue Dec 20, 2022 · 3 comments · Fixed by #2579
Assignees
Milestone

Comments

@philipphofmann
Copy link
Member

philipphofmann commented Dec 20, 2022

Description

Undo #2404 because hybrid SDKs need to add now the code we removed with #2404; see React-Native. Make initWithDict only visible to hybrid SDKs though via PrivateSentrySDKOnly.

RN are the only ones using initWithDict at the moment. Maybe it's better to move the code removed in #2404 to the RN repo including tests.

@philipphofmann philipphofmann added this to the 8.0.0 milestone Dec 20, 2022
@philipphofmann philipphofmann changed the title Undo SDK initWithDict removal Undo Options initWithDict removal Dec 20, 2022
@kevinrenskers
Copy link
Contributor

If RN is the only one now using this code, it makes sense to me that they added the code we removed. Why add it back here?

(Also, why don't they just use the options object and set properties, instead of working with the dict?)

@philipphofmann
Copy link
Member Author

Cordova and also Unity use initWithDict. So we would need to add the code we removed in 3 different places. Therefore, I would rather add it again here.

@kevinrenskers
Copy link
Contributor

That makes sense. But why do they even use it in the first place?

@philipphofmann philipphofmann self-assigned this Jan 4, 2023
philipphofmann added a commit that referenced this issue Jan 5, 2023
Add SentryOptions:initWithDict only for Hybrid SDKs again.

Fixes GH-2547
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants