Skip to content

RUMM-2542 Send hasReplay property for RUM events#1054

Merged
mariusc83 merged 1 commit into
feature/session-replay-vofrom
mconstantin/rumm-2542/send-has-replay-true-for-rum-when-recording
Sep 26, 2022
Merged

RUMM-2542 Send hasReplay property for RUM events#1054
mariusc83 merged 1 commit into
feature/session-replay-vofrom
mconstantin/rumm-2542/send-has-replay-true-for-rum-when-recording

Conversation

@mariusc83

Copy link
Copy Markdown
Member

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

codecov-commenter commented Sep 23, 2022

Copy link
Copy Markdown

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

@0xnm 0xnm left a comment

Copy link
Copy Markdown
Member

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.

assertThat(testedCore.webViewRumFeature).isNull()
assertThat(testedCore.crashReportsFeature).isNull()
assertThat(testedCore.sessionReplayFeature).isNull()
assertThat(testedCore.contextProvider).isNull()

Copy link
Copy Markdown
Member

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.

4 participants