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.

Sign up for GitHub

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

Jump to bottom

feat: segment compatibility #685

Merged
merged 18 commits into from
May 7, 2024
Merged

feat: segment compatibility #685

merged 18 commits into from
May 7, 2024

Conversation

Shahroz16
Copy link
Contributor

@Shahroz16 Shahroz16 commented Apr 1, 2024 •
edited
Loading

closes: https://linear.app/customerio/issue/MBL-280/rename-symbols

Problem:
Our fork and Segment SDK can't be added at the same time via SPM.

Solution:
Update module name from Segment to CioAnalytics in fork.

Changes in this PR:

  • Change all the references and imports from Segment to CDPAnalyticsSwift in SDK and sample app.

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 1, 2024 •
edited
Loading

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.

  • CocoaPods-FCM: shahroz/segment-compatibility (1715078116)
  • APN-UIKit: shahroz/segment-compatibility (1715078125)

@CarlosModinez
Copy link

Thank you for that. I was struggling with the exact same error

@Shahroz16
Copy link
Contributor Author

@CarlosModinez we appreciate the patience, for clarity, work is still ongoing on this we want to make sure we get it right not just for now but for future compatibility too.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 6, 2024 •
edited
Loading

Warnings
⚠️

I noticed file Tests/DataPipeline/APITest.swift was modified. That could mean that this pull request is introducing a breaking change to the SDK.

If this pull request does introduce a breaking change, make sure the pull request title is in the format:

<type>!: description of breaking change 
// Example:
refactor!: remove onComplete callback from async functions

Generated by 🚫 dangerJS against 4641671

@codecov Codecov
Copy link

codecov bot commented May 7, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.93%. Comparing base (4c4af15) to head (4641671).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #685      +/-   ##
==========================================
- Coverage   56.96%   56.93%   -0.03%     
==========================================
  Files         139      139              
  Lines        3869     3869              
==========================================
- Hits         2204     2203       -1     
- Misses       1665     1666       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Shahroz16 Shahroz16 marked this pull request as ready for review May 7, 2024 10:27
Shahroz16
Shahroz16 commented May 7, 2024
View reviewed changes
Sources/DataPipeline/Type/Aliases.swift
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We no longer need to facilitate the use case for users to utilize Segment classes rather have separate classes now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Because we don't have listed these APIs anywhere in our docs, I'm okay removing this as I don't expect the change to break project for customers both with or without analytics library.

@Shahroz16 Shahroz16 changed the title fix: segment compatibility feat: segment compatibility May 7, 2024
@Shahroz16 Shahroz16 requested a review from a team May 7, 2024 10:37
mrehan27
mrehan27 approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@mrehan27 mrehan27 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 👍🏻

Sources/DataPipeline/Type/Aliases.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

Because we don't have listed these APIs anywhere in our docs, I'm okay removing this as I don't expect the change to break project for customers both with or without analytics library.

@Shahroz16 Shahroz16 merged commit 07dc0e5 into main May 7, 2024
12 of 13 checks passed
@Shahroz16 Shahroz16 deleted the shahroz/segment-compatibility branch May 7, 2024 14:35
Shahroz16 added a commit that referenced this pull request May 7, 2024
@Shahroz16
feat: segment compatibility (#685)
7298783
github-actions bot pushed a commit that referenced this pull request May 7, 2024
@semantic-release-bot
chore: prepare for 3.2.0
490593e 
## [3.2.0](3.1.3...3.2.0) (2024-05-07)

### Features

* segment compatibility ([#685](#685)) ([7298783](7298783))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrehan27 mrehan27 mrehan27 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Shahroz16 @CarlosModinez @mrehan27