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-1160 Static pod support #451

Merged
merged 3 commits into from
Apr 9, 2021

Conversation

buranmert
Copy link
Contributor

@buranmert buranmert commented Mar 26, 2021

What and why?

DatadogSDK.podspec requires !use_frameworks in client's Podfile
Lack of it results in error at pod install

How?: A proposal

For cocoapods and carthage, I can mix Swift and ObjC by importing ObjC headers in framework's umbrella header as Apple suggests

For Swift Package Manager, this is not possible as mixed targets are not supported.
To solve this, I pass SPM_BUILD flag from Package.swift where we create _Datadog_Private target.
Now I can #if SPM_BUILD import _Datadog_Private #endif in the source code.

Review checklist

  • Rename ObjC methods so that clients will notice that they are not for public use
  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

@buranmert buranmert self-assigned this Mar 26, 2021
@buranmert buranmert force-pushed the buranmert/RUMM-1160-support-static-pod branch from eb13532 to 65d86fc Compare March 27, 2021 13:22
@buranmert buranmert force-pushed the buranmert/RUMM-1160-support-static-pod branch from 65d86fc to fafc060 Compare April 7, 2021 16:51
@buranmert
Copy link
Contributor Author

buranmert commented Apr 7, 2021

✅ CI passed with use_frameworks
🗣️ now i'm pushing a commit to remove use_frameworks

@buranmert buranmert marked this pull request as ready for review April 7, 2021 17:55
@buranmert buranmert requested a review from a team as a code owner April 7, 2021 17:55
@buranmert buranmert force-pushed the buranmert/RUMM-1160-support-static-pod branch from 5d85db7 to 9f9584b Compare April 7, 2021 19:22
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.

Looks good 👌 💪 , let's just remove the Package.resolved.

Package.resolved Outdated
@@ -0,0 +1,25 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

This file should be git-ignored 🧽

Those methods renamed to `__dd_private_MethodName` to discourage users
@buranmert buranmert requested a review from ncreated April 8, 2021 14:50
@buranmert buranmert merged commit af85391 into master Apr 9, 2021
@buranmert buranmert deleted the buranmert/RUMM-1160-support-static-pod branch April 9, 2021 08:28
buranmert added a commit that referenced this pull request Apr 12, 2021
@buranmert buranmert mentioned this pull request Apr 12, 2021
3 tasks
buranmert added a commit that referenced this pull request Apr 12, 2021
ncreated pushed a commit that referenced this pull request Apr 13, 2021
ncreated pushed a commit that referenced this pull request Apr 13, 2021
@ncreated ncreated mentioned this pull request Apr 13, 2021
2 tasks
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.

2 participants