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

Fix Session Replay ObjC interface #2077

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Fix Session Replay ObjC interface #2077

merged 2 commits into from
Oct 10, 2024

Conversation

mariedm
Copy link
Member

@mariedm mariedm commented Oct 4, 2024

What and why?

The current Objective-C interface for Session Replay doesn't adhere to the internal guidelines for Objective-C interoperability (see RFC).

How?

  • Applied @_spi(objc) to hide the ObjC interfaces from Swift consumers while keeping them accessible to ObjC clients.
  • Prefixed types with objc_* to differentiate them from Swift-native types.
  • Updated the ObjC interface for SessionReplay, SessionReplayConfiguration, and the different privacy levels following the modular interface guidelines.
  • Moved DDSessionReplayTests to the Session Replay module

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

@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Oct 4, 2024

Datadog Report

Branch report: fix-sr-objc-interface
Commit report: ce213b9
Test service: dd-sdk-ios

✅ 0 Failed, 3479 Passed, 0 Skipped, 2m 21.39s Total Time
🔻 Test Sessions change in coverage: 5 decreased, 4 increased, 5 no change

🔻 Code Coverage Decreases vs Default Branch (5)

  • test DatadogCoreTests iOS 72.58% (-0.16%) - Details
  • test DatadogTraceTests tvOS 54.29% (-0.04%) - Details
  • test DatadogInternalTests iOS 79.47% (-0.04%) - Details
  • test DatadogRUMTests iOS 81.08% (-0.03%) - Details
  • test DatadogTraceTests iOS 54.25% (-0.02%) - Details

@mariedm mariedm marked this pull request as ready for review October 7, 2024 08:28
@mariedm mariedm requested review from a team as code owners October 7, 2024 08:28
@0xnm
Copy link
Member

0xnm commented Oct 7, 2024

Is the following a potentially backward-incompatible change?

Prefixed types with objc_* to differentiate them from Swift-native types.

It is changing the name of the exported symbols, right?

@mariedm
Copy link
Member Author

mariedm commented Oct 7, 2024

Is the following a potentially backward-incompatible change?

Prefixed types with objc_* to differentiate them from Swift-native types.

It is changing the name of the exported symbols, right?

This won't cause any backward compatibility issues. ObjC users will still use DDSessionReplay and and DDSessionReplayConfiguration thanks to the @objc(xxx) annotation. The exported symbols will stay the same.

@mariedm mariedm force-pushed the fix-sr-objc-interface branch from bb39dca to ce213b9 Compare October 7, 2024 14:38
Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

👍 Happy to see this cleanup, well done.

@mariedm mariedm merged commit 965f1e0 into develop Oct 10, 2024
15 checks passed
@mariedm mariedm deleted the fix-sr-objc-interface branch October 10, 2024 11:07
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