address event plugin disparity and track issue #87
Conversation
Codecov Report
@@ Coverage Diff @@
## main #87 +/- ##
=============================================
+ Coverage 45.49% 77.69% +32.19%
- Complexity 14 440 +426
=============================================
Files 8 64 +56
Lines 855 5487 +4632
Branches 85 692 +607
=============================================
+ Hits 389 4263 +3874
- Misses 436 679 +243
- Partials 30 545 +515
Continue to review full report at Codecov.
|
| result = plugin.alias(result as AliasEvent) | ||
| } | ||
| } | ||
| plugin.execute(result!!) |
There was a problem hiding this comment.
i think this needs to be result = plugin.execute(result)
secondly, we should not be using !! since the result from one of the plugins could be null and this would then throw an NPE. lets wrap the call in a null check or use ?.let and capture the state of result before using it as non-null
There was a problem hiding this comment.
hrm. result = plugin.execute(result) is a compile time error. we don't need a null check because we checked it on line 27
There was a problem hiding this comment.
actually scratch that first comment, didnt realize this was for DestinationPlugin, but second one still holds
| } | ||
| else -> { | ||
| result = plugin.execute(result as BaseEvent) | ||
| result = plugin.execute(result!!) |
| @@ -70,6 +70,14 @@ interface EventPlugin : Plugin { | |||
| return payload | |||
| } | |||
|
|
|||
| override fun execute(event: BaseEvent): BaseEvent? = when (event) { | |||
There was a problem hiding this comment.
should this be marked final?
There was a problem hiding this comment.
no. we shouldn't, because we extends execute in edge function. amplitude also overrides this method.
|
I'd say we're pretty safe here. the difference lays on whether the child plugin overrides execute method and if the execute method calls |
executedisparity between swift and kotlin