Skip to content

Fix _metadata population #114

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

Merged
merged 3 commits into from
Oct 3, 2022
Merged

Fix _metadata population #114

merged 3 commits into from
Oct 3, 2022

Conversation

prayansh
Copy link
Contributor

@prayansh prayansh commented Oct 3, 2022

Our previous implementation was lacking a little, causing issues with sending data to destinations on cloud-mode.

These changes should now incorporate the list of active destinations and mark them as unbundled if no device-mode equivalent is present.

Copy link
Contributor

@wenxi-zeng wenxi-zeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. couple minor optimizations:

  1. make bundled as a set first. so bundled.contains becomes an O(1) operation, and filter + contains is O(n). otherwise it is O(mn).
  2. make unbundled as a set first. so we don't have to do a union at the end, saves m+n operations, otherwise, it's 2*(m+n).

@codecov-commenter
Copy link

Codecov Report

Base: 78.00% // Head: 83.84% // Increases project coverage by +5.83% 🎉

Coverage data is based on head (25dc8a0) compared to base (faf431b).
Patch coverage: 85.71% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #114      +/-   ##
============================================
+ Coverage     78.00%   83.84%   +5.83%     
+ Complexity      442      429      -13     
============================================
  Files            64       56       -8     
  Lines          5705     4901     -804     
  Branches        719      639      -80     
============================================
- Hits           4450     4109     -341     
+ Misses          693      256     -437     
+ Partials        562      536      -26     
Impacted Files Coverage Δ
...core/platform/plugins/DestinationMetadataPlugin.kt 81.25% <73.33%> (+9.82%) ⬆️
...platform/plugins/DestinationMetadataPluginTests.kt 91.17% <90.24%> (-0.63%) ⬇️
...analytics/kotlin/android/AndroidAnalyticsKtTest.kt
.../analytics/kotlin/android/utils/MockPreferences.kt
.../segment/analytics/kotlin/android/utils/Plugins.kt
...m/segment/analytics/kotlin/android/StorageTests.kt
...ics/kotlin/android/AndroidContextCollectorTests.kt
...egment/analytics/kotlin/android/EventsFileTests.kt
...om/segment/analytics/kotlin/android/utils/Mocks.kt
...tics/kotlin/android/AndroidLifecyclePluginTests.kt

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@prayansh prayansh merged commit 3bd6a19 into main Oct 3, 2022
@prayansh prayansh deleted the pray/fix-metadata branch October 3, 2022 20:55
@prayansh prayansh mentioned this pull request Oct 6, 2022
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.

3 participants