RUM-2485 Provide the parent container information for browser rum events#1831
Merged
Conversation
mariusc83
force-pushed
the
mconstantin/rum-2875/propagate-feature-context-update-through-msb
branch
2 times, most recently
from
January 19, 2024 16:44
4ba06aa to
0011b35
Compare
0xnm
reviewed
Jan 26, 2024
| private fun resolveRumFeature(sdkCore: FeatureSdkCore): WebViewRumFeature? { | ||
| ( | ||
| sdkCore.getFeature(WebViewRumFeature.WEB_RUM_FEATURE_NAME) | ||
| ?.unwrap<StorageBackedFeature>() as? WebViewRumFeature |
Member
There was a problem hiding this comment.
Is there a particular reason for doing ?.unwrap<StorageBackedFeature>() as? WebViewRumFeature instead of the ?.unwrap<WebViewRumFeature>()? Same for the similar calls below.
Member
Author
There was a problem hiding this comment.
oh...nope I think I did that just because I've seen it that way in another place. Better how you suggested
Member
Author
There was a problem hiding this comment.
I found the explanation why, is because in the core we are force casting it, we are not using as? :
override fun <T : Feature> unwrap(): T = wrappedFeature as T
Member
There was a problem hiding this comment.
but it is us who is owning implementation anyway, so we if got the scope for the feature, we can safely unwrap with using as under the hood.
| } | ||
|
|
||
| private fun addToCache( | ||
| entry: Triple<String, Long, Boolean> |
Member
There was a problem hiding this comment.
I would suggest to create a dedicated class maybe for that.
mariusc83
force-pushed
the
mconstantin/rum-2875/propagate-feature-context-update-through-msb
branch
2 times, most recently
from
January 29, 2024 09:10
6869dff to
474b8b0
Compare
mariusc83
force-pushed
the
mconstantin/rum-2485/provide-the-container-data-into-browser-rum-events
branch
from
January 29, 2024 09:26
392479d to
5a770f1
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## feature/sr-web-view-support #1831 +/- ##
===============================================================
- Coverage 83.69% 83.68% -0.01%
===============================================================
Files 472 473 +1
Lines 16615 16706 +91
Branches 2504 2533 +29
===============================================================
+ Hits 13905 13979 +74
- Misses 2015 2019 +4
- Partials 695 708 +13
|
Base automatically changed from
mconstantin/rum-2875/propagate-feature-context-update-through-msb
to
feature/sr-web-view-support
January 30, 2024 07:57
mariusc83
force-pushed
the
mconstantin/rum-2485/provide-the-container-data-into-browser-rum-events
branch
3 times, most recently
from
January 30, 2024 09:51
0734db6 to
527454f
Compare
mariusc83
marked this pull request as ready for review
January 30, 2024 10:13
xgouchet
previously approved these changes
Jan 30, 2024
mariusc83
force-pushed
the
mconstantin/rum-2485/provide-the-container-data-into-browser-rum-events
branch
3 times, most recently
from
January 31, 2024 15:41
243ea1e to
952e57a
Compare
mariusc83
force-pushed
the
mconstantin/rum-2485/provide-the-container-data-into-browser-rum-events
branch
from
January 31, 2024 16:06
952e57a to
688c2a7
Compare
xgouchet
approved these changes
Feb 1, 2024
mariusc83
deleted the
mconstantin/rum-2485/provide-the-container-data-into-browser-rum-events
branch
February 1, 2024 13:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
A brief description of the change being made with this pull request.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)