Skip to content

RUM-7205 Double full snapshot bug#2154

Merged
dd-mergequeue[bot] merged 3 commits into
developfrom
maciey/RUM-7205/double-full-snapshot-bug
Dec 18, 2024
Merged

RUM-7205 Double full snapshot bug#2154
dd-mergequeue[bot] merged 3 commits into
developfrom
maciey/RUM-7205/double-full-snapshot-bug

Conversation

@maciejburda

@maciejburda maciejburda commented Dec 18, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

What and why?

This PR is essentially reverting #1567. Assumption made was wrong. We can expect view that from the wireframe perspective have a different root view (it can be done by fully covering the root with nontransparent view, which will remove the root during flattening process).

The reverted code was fixing some minor edge cases in React Native, although it has a serious penalty of creating double fullsnapshots in others.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes
  • Add Objective-C interface for public APIs (see our guidelines [internal]) and run make api-surface)

@maciejburda
maciejburda requested review from a team as code owners December 18, 2024 14:43
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Dec 18, 2024 •
edited
Loading

Copy link
Copy Markdown

Datadog Report

Branch report: maciey/RUM-7205/double-full-snapshot-bug
Commit report: f1afae5
Test service: dd-sdk-ios

✅ 0 Failed, 2 Passed, 3591 Skipped, 16.62s Total duration (2m 15.09s time saved)

@maciejburda

maciejburda commented Dec 18, 2024

Copy link
Copy Markdown
Member Author

/merge

@dd-devflow

dd-devflow Bot commented Dec 18, 2024 •
edited
Loading

Copy link
Copy Markdown

Devflow running: /merge

View all feedbacks in Devflow UI.

2024-12-18 15:13:20 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-12-18 16:33:46 UTC ℹ️ MergeQueue: merge request added to the queue

The median merge time in develop is 28m.

2024-12-18 17:30:45 UTC 🚨 MergeQueue: This merge request is in error

mergequeue build completed successfully, but the github api returned an error while merging the pr.
It's probably because:

  • target branch of PR is restricted to only allow up-to-date branches, but the pr is now outdated
Details

Error: PUT https://api.github.com/repos/DataDog/dd-sdk-ios/pulls/2154/merge: 405 Required status check "dd-gitlab/Sync GH Checks" is expected. []

FullStacktrace:
activity error (type: github.GithubService_MergePullRequest, scheduledEventID: 41, startedEventID: 42, identity: 1@github-worker-7c5557687b-pw9fq@): PUT https://api.github.com/repos/DataDog/dd-sdk-ios/pulls/2154/merge: 405 Required status check "dd-gitlab/Sync GH Checks" is expected. [] (type: GitFailure, retryable: false): PUT https://api.github.com/repos/DataDog/dd-sdk-ios/pulls/2154/merge: 405 Required status check "dd-gitlab/Sync GH Checks" is expected. [] (type: ErrorResponse, retryable: true)

ncreated
ncreated approved these changes Dec 18, 2024
View reviewed changes

@ncreated ncreated 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.

Good finding, good explanation, good trade-off 💪

@maxep
maxep force-pushed the maciey/RUM-7205/double-full-snapshot-bug branch from f1afae5 to fd28491 Compare December 18, 2024 17:32
@maxep

maxep commented Dec 18, 2024

Copy link
Copy Markdown
Member

/merge

@dd-devflow

dd-devflow Bot commented Dec 18, 2024 •
edited
Loading

Copy link
Copy Markdown

Devflow running: /merge

View all feedbacks in Devflow UI.

2024-12-18 17:32:44 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-12-18 18:03:11 UTC ℹ️ MergeQueue: merge request added to the queue

The median merge time in develop is 28m.

2024-12-18 18:29:56 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue
dd-mergequeue Bot merged commit 5a49e0f into develop Dec 18, 2024
@mariedm
mariedm deleted the maciey/RUM-7205/double-full-snapshot-bug branch December 19, 2024 07:57
@maciejburda maciejburda mentioned this pull request Jan 2, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ncreated ncreated ncreated approved these changes
@maxep maxep maxep approved these changes
@mariedm mariedm mariedm approved these changes

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maciejburda @maxep @ncreated @mariedm