-
-
Notifications
You must be signed in to change notification settings - Fork 205
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
refactor: rename native options #2940
Changes from all commits
b952cb0
98f7b57
8c4854f
447d12e
3caf34f
640d94d
163aaa2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,17 +9,17 @@ internal class AndroidEventProcessor : ISentryEventProcessor, IDisposable | |
private readonly JavaSdk.IEventProcessor? _androidProcessor; | ||
private readonly JavaSdk.Hint _hint = new(); | ||
|
||
public AndroidEventProcessor(SentryAndroidOptions androidOptions) | ||
public AndroidEventProcessor(SentryAndroidOptions nativeOptions) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should class Although I see it being renamed below, did we have 2 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This refers to Java class name |
||
{ | ||
// Locate the Android SDK's default event processor by its class | ||
// NOTE: This approach avoids hardcoding the class name (which could be obfuscated by proguard) | ||
_androidProcessor = androidOptions.EventProcessors.OfType<JavaObject>() | ||
_androidProcessor = nativeOptions.EventProcessors.OfType<JavaObject>() | ||
.Where(o => o.Class == JavaClass.FromType(typeof(DefaultAndroidEventProcessor))) | ||
.Cast<JavaSdk.IEventProcessor>() | ||
.FirstOrDefault(); | ||
|
||
// TODO: This would be cleaner, but doesn't compile. Figure out why. | ||
// _androidProcessor = androidOptions.EventProcessors | ||
// _androidProcessor = nativeOptions.EventProcessors | ||
// .OfType<DefaultAndroidEventProcessor>() | ||
// .FirstOrDefault(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,7 +6,7 @@ | |||||
|
||||||
<ItemGroup> | ||||||
<Using Include="Foundation" /> | ||||||
<Using Include="Sentry.CocoaSdk.SentryOptions" Alias="SentryCocoaOptions" /> | ||||||
<Using Include="Sentry.CocoaSdk.SentryOptions" Alias="SentryCocoaSdkOptions" /> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about?
Suggested change
Not sure the namespace is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is related to the native cocoa SDK generated binding code. I didn't want to touch that internal bit |
||||||
<Using Include="Sentry.CocoaSdk.SentrySDK" Alias="SentryCocoaSdk" /> | ||||||
<Using Include="Sentry.CocoaSdk.PrivateSentrySDKOnly" Alias="SentryCocoaHybridSdk" /> | ||||||
</ItemGroup> | ||||||
|
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.
Note to final 4.0.0 changelog rewording: outside the beta there was no
Cocoa
, it wasiOS
toNative
(right?)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.
Yes and that is also true for many other parts of the 4.0 changelog. we need to update it and review thoroughly before the final release.
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.
We could keep track of these things in a ticket (we already have #2673)?