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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[iOS][amplitude] Upgrade Amplitude-iOS@4.7.1 to Amplitude@6.0.0 #9880

Merged
merged 4 commits into from Aug 24, 2020
Merged

[iOS][amplitude] Upgrade Amplitude-iOS@4.7.1 to Amplitude@6.0.0 #9880

merged 4 commits into from Aug 24, 2020

Conversation

bbarthec
Copy link
Contributor

Why

  • Amplitude-iOS changed it's name to Amplitude
  • Amplitude v6.0.0 introduced accommodation for new rules over IDFA in iOS 14

How

  • Changed Amplitude-iOS ~> v4.7.1 to Amplitude ~> 6.0.0 across whole iOS project (versioned pods included).

Test Plan

  • Expo Client compiles and runs
  • Home that uses Amplitude works without crashes (home is the only app that actually uses Amplitude package)
    • locally launched home hans't floated with any error coming form expo-analytics-amplitude after this upgrade 馃槄

Changelog

  • [feat] Upgraded native Amplitude iOS library.

@github-actions github-actions bot mentioned this pull request Aug 21, 2020
Merged
@github-actions
Copy link
Contributor

Native Component List for this branch is ready

tsapeta
tsapeta approved these changes Aug 21, 2020
View reviewed changes
Copy link
Member

@tsapeta tsapeta left a comment

Choose a reason for hiding this comment

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

馃憤
Did you make sure we don't need to run pod install in bare-expo?
Could you please add a note about this upgrade to expo-analytics-amplitude changelog?

bbarthec pushed a commit that referenced this pull request Aug 21, 2020
@github-actions
[danger][bot] Add missing changelog to #9880 (#9881)
34a4f60
tsapeta
tsapeta approved these changes Aug 21, 2020
View reviewed changes
packages/expo-analytics-amplitude/CHANGELOG.md Outdated
@@ -6,6 +6,8 @@

### 馃帀 New features

- Upgraded native Amplitude iOS library from `4.7.1` to `6.0.0`. ([#9880](https://github.com/expo/expo/pull/9880) by [@bbarthec](https://github.com/bbarthec))
Copy link
Member

Choose a reason for hiding this comment

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

What do you think if we also mention about changes with IDFA?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

50/50 馃槈 I don't see a reason for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should mention it, I added a note about the same removal for the segment library, because this is a breaking change that users should know about. If they want want to collect the IDFA, they need to use the bare workflow now.

cruzach
cruzach suggested changes Aug 21, 2020
View reviewed changes
Copy link
Contributor

@cruzach cruzach left a comment •
edited

Choose a reason for hiding this comment

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

Thank you!! glad we are definitely going to be safe and avoid any potential app rejections because of this 馃槃

this upgrade also "Removed disableIdfaTracking API." so I think we should remove that option here and also remove it from the docs.

packages/expo-analytics-amplitude/CHANGELOG.md Outdated
@@ -6,6 +6,8 @@

### 馃帀 New features

- Upgraded native Amplitude iOS library from `4.7.1` to `6.0.0`. ([#9880](https://github.com/expo/expo/pull/9880) by [@bbarthec](https://github.com/bbarthec))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should mention it, I added a note about the same removal for the segment library, because this is a breaking change that users should know about. If they want want to collect the IDFA, they need to use the bare workflow now.

sjchmiela
sjchmiela approved these changes Aug 21, 2020
View reviewed changes
Copy link
Contributor

@sjchmiela sjchmiela left a comment

Choose a reason for hiding this comment

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

Apart from the missing breaking change log looks good to me! 馃憤

@bbarthec bbarthec mentioned this pull request Aug 24, 2020
Merged
1 task
bbarthec and others added 4 commits August 24, 2020 11:48
@bbarthec bbarthec force-pushed the @bbarthec/upgrade-amplitude-ios branch from 989033f to ed3d8ba Compare August 24, 2020 09:59
@bbarthec bbarthec merged commit 7b76bab into master Aug 24, 2020
@bbarthec bbarthec deleted the @bbarthec/upgrade-amplitude-ios branch August 24, 2020 11:34
bbarthec added a commit that referenced this pull request Aug 24, 2020
@bbarthec
[iOS][amplitude] Upgrade Amplitude-iOS@4.7.1 to Amplitude@6.0.0 (#9880)
a31e19e 
# Why
Amplitude-iOS changed it's name to Amplitude
Amplitude v6.0.0 introduced accommodation for new rules over IDFA in iOS 14
# How
Changed Amplitude-iOS ~> v4.7.1 to Amplitude ~> 6.0.0 across whole iOS project (versioned pods included).
cruzach added a commit that referenced this pull request Aug 24, 2020
@cruzach
[docs] No more "check the IDFA box" in SDK 39
bd64e50 
Removal of IDFA code in these PRs:
- #9606
- #9880
bbarthec pushed a commit that referenced this pull request Aug 25, 2020
@cruzach @bbarthec
[docs] No more "check the IDFA box" in SDK 39
647fa25 
Removal of IDFA code in these PRs:
- #9606
- #9880
This was referenced Sep 15, 2021
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Sep 16, 2021
Open
Open
Open
Open
Open
@snyk-bot snyk-bot mentioned this pull request Dec 16, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cruzach cruzach cruzach requested changes

@sjchmiela sjchmiela sjchmiela approved these changes

@tsapeta tsapeta tsapeta approved these changes

@esamelson esamelson Awaiting requested review from esamelson

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@bbarthec @sjchmiela @tsapeta @cruzach