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

RUMM-2542 Send hasReplay property for RUM events #1054

Conversation

mariusc83
Copy link
Collaborator

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)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@mariusc83 mariusc83 self-assigned this Sep 23, 2022
@mariusc83 mariusc83 force-pushed the mconstantin/rumm-2542/send-has-replay-true-for-rum-when-recording branch 2 times, most recently from 9de000f to 580b8d0 Compare September 23, 2022 13:51
@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2022

Codecov Report

Merging #1054 (99d17a7) into feature/session-replay-vo (a710a59) will increase coverage by 0.08%.
The diff coverage is 89.17%.

@@                      Coverage Diff                      @@
##           feature/session-replay-vo    #1054      +/-   ##
=============================================================
+ Coverage                      83.26%   83.35%   +0.08%     
=============================================================
  Files                            332      343      +11     
  Lines                          10569    10791     +222     
  Branches                        1773     1792      +19     
=============================================================
+ Hits                            8800     8994     +194     
- Misses                          1219     1243      +24     
- Partials                         550      554       +4     
Impacted Files Coverage Δ
...main/kotlin/com/datadog/android/DatadogEndpoint.kt 0.00% <ø> (ø)
...nal/tracking/AndroidXFragmentLifecycleCallbacks.kt 93.48% <ø> (ø)
...otlin/com/datadog/android/v2/api/RequestFactory.kt 0.00% <0.00%> (ø)
...id/v2/core/internal/storage/ConsentAwareStorage.kt 96.81% <ø> (ø)
...atadog/android/v2/core/internal/storage/Storage.kt 0.00% <ø> (ø)
...atadog/android/sessionreplay/NoOpRecordCallback.kt 0.00% <0.00%> (ø)
...og/android/v2/core/internal/NoOpContextProvider.kt 4.35% <4.35%> (ø)
...id/sessionreplay/internal/NoOpLifecycleCallback.kt 10.00% <10.00%> (ø)
...src/main/kotlin/com/datadog/android/DatadogSite.kt 83.33% <16.67%> (-3.33%) ⬇️
...oid/src/main/kotlin/com/datadog/android/Datadog.kt 72.55% <50.00%> (ø)
... and 89 more

@mariusc83 mariusc83 force-pushed the mconstantin/rumm-2542/send-has-replay-true-for-rum-when-recording branch 2 times, most recently from 1dc5b2f to 99d17a7 Compare September 23, 2022 14:26
@mariusc83 mariusc83 marked this pull request as ready for review September 23, 2022 14:32
@mariusc83 mariusc83 requested a review from a team as a code owner September 23, 2022 14:32
@xgouchet xgouchet added the size-medium This PR is medium sized label Sep 23, 2022
Copy link
Contributor

@0xnm 0xnm left a comment

Choose a reason for hiding this comment

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

lgtm. I just added one comment regarding the assertion removed from the test.

@@ -286,7 +286,6 @@ internal class DatadogCoreTest {
assertThat(testedCore.webViewRumFeature).isNull()
assertThat(testedCore.crashReportsFeature).isNull()
assertThat(testedCore.sessionReplayFeature).isNull()
assertThat(testedCore.contextProvider).isNull()
Copy link
Contributor

Choose a reason for hiding this comment

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

why this line is removed? In the latest version of the SDK v2 branch at least when CoreFeature is stopped, contextProvider should return null.

@mariusc83 mariusc83 force-pushed the mconstantin/rumm-2542/send-has-replay-true-for-rum-when-recording branch from 99d17a7 to 76ec178 Compare September 26, 2022 07:45
@mariusc83 mariusc83 merged commit e840667 into feature/session-replay-vo Sep 26, 2022
@mariusc83 mariusc83 deleted the mconstantin/rumm-2542/send-has-replay-true-for-rum-when-recording branch September 26, 2022 09:17
@xgouchet xgouchet added this to the internal milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size-medium This PR is medium sized
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants