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

[WEB-740] Refactored AppBoySegment to SPM #1735

Merged
merged 3 commits into from Sep 21, 2022

Conversation

msadoon
Copy link
Contributor

@msadoon msadoon commented Sep 20, 2022

πŸ“² What

Refactor of this pr so no code changes are required.

πŸ€” Why

Wanted to ensure we had as minimal code changes as necessary.

πŸ›  How

  1. Removed github "appboy/appboy-segment-ios" == 4.0.0 from Cartfile and Cartfile.resolved
  2. Removed Segment.framework and Segment_Appboy.framework from app-input-files app-output-files library-input-files library-output-files
  3. Removed all dependencies from project.pbxproj
Build Phases - Link Binary With Libraries
Kickstarter-Framework-iOSTests
Segment_Appboy.framework

Library
Segment.framework
Segment_Appboy.framework

Library-iOSTests
Segment_Appboy.framework

Kickstarter-iOS
Segment.framework
Segment_Appboy.framework

Build Phases - Copy Files

Library-Framework-iOSTests
Segment.framework
Segment_Appboy.framework

Kickstarter-Framework-iOSTests
Segment.framework
Segment_Appboy.framework
  1. Used https://github.com/Appboy/appboy-segment-ios in SPM, only pulled in AppboySegment not AppboySegmentCore.

  2. Only added AppboySegment to Kickstarter-FrameworkiOS and Library, nowehere else.

  3. Made minor import corrections in

AppDelegate, BrazeTypes, 
import Segment_Appboy --> import AppboySegment

KSRAnalytics
import Segment --> import AppboySegment

Segment
import Segment, import Segment_Appboy, import Appboy_iOS_SDK --> import AppboySegment

βœ… Acceptance criteria

  • See that segment events are still being sent
  • Ensure Braze pn's and in-app messages are still working.

@msadoon msadoon self-assigned this Sep 20, 2022
@msadoon msadoon added this to the release-5.5.0 milestone Sep 20, 2022
@msadoon msadoon marked this pull request as ready for review September 20, 2022 20:54
@codecov
Copy link

codecov bot commented Sep 20, 2022

Codecov Report

Merging #1735 (cc22e06) into main (6c367db) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #1735   +/-   ##
=======================================
  Coverage   85.35%   85.35%           
=======================================
  Files        1270     1270           
  Lines      114842   114841    -1     
  Branches    30335    30335           
=======================================
  Hits        98028    98028           
+ Misses      15762    15761    -1     
  Partials     1052     1052           
Impacted Files Coverage Ξ”
Library/Tracking/KSRAnalytics.swift 81.76% <ΓΈ> (ΓΈ)
Library/Tracking/Segment.swift 0.00% <0.00%> (ΓΈ)

πŸ“£ We’re building smart automated test selection to slash your CI/CD build times. Learn more

@msadoon msadoon assigned scottkicks and msadoon and unassigned msadoon Sep 20, 2022
@msadoon msadoon removed the request for review from scottkicks September 20, 2022 21:48
@msadoon
Copy link
Contributor Author

msadoon commented Sep 20, 2022

crashing on launch on device, one sec, maybe this does require more code changes.

@msadoon msadoon marked this pull request as draft September 20, 2022 21:49
@msadoon msadoon marked this pull request as ready for review September 20, 2022 23:09
@msadoon
Copy link
Contributor Author

msadoon commented Sep 20, 2022

crashing on launch on device, one sec, maybe this does require more code changes.

No longer issue, last commit resolves a weird threading issue and prevents duplicate configuration on Analytics

Comment on lines -21 to -23
Analytics.setup(with: configuration)

return Analytics.shared()
Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch

@msadoon msadoon merged commit 514fde9 into main Sep 21, 2022
@msadoon msadoon deleted the fix/segment-appboy-to-spm-check branch September 21, 2022 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants