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

[iOS][expo-sms] migrate expo-sms to new modules api #19996

Merged
merged 12 commits into from Dec 6, 2022

Conversation

alanjhughes
Copy link
Collaborator

Why

Continue migration of expo modules to the new API

How

Followed the usual steps involved in migrating a module

Test Plan

All original tests pass as before with a minor modification. Changed Constants.isDevice() to Device.isDevice(). Also, one incidence where all tests failed and then passed after that. Maybe @tsapeta will be able to see an issue somewhere?

IMG_ACDEC7B393AD-1

@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Nov 11, 2022
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.

Great work! Here is the first round of the review 🙂
Could you also reindent all Swift files? We use 2 spaces, not 4 (Xcode's default)

packages/expo-sms/CHANGELOG.md Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/SMSDelegate.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/SMSOptions.swift Outdated Show resolved Hide resolved
@alanjhughes
Copy link
Collaborator Author

I'll get started on these soon 👍

apps/bare-expo/ios/BareExpo/Info.plist Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/SMSOptions.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/SMSDelegate.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/ExpoSMSModule.swift Outdated Show resolved Hide resolved
packages/expo-sms/ios/SMSDelegate.swift Outdated Show resolved Hide resolved
@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions bot: passed checks ExpoBot has nothing to complain about and removed bot: passed checks ExpoBot has nothing to complain about bot: suggestions ExpoBot has some suggestions labels Nov 28, 2022
@alanjhughes alanjhughes requested review from tsapeta and removed request for brentvatne December 2, 2022 17:41
packages/expo-sms/CHANGELOG.md Outdated Show resolved Hide resolved
@tsapeta tsapeta merged commit c08955c into expo:main Dec 6, 2022
@alanjhughes alanjhughes deleted the @alanhughes/ios/sweet-expo-sms branch December 6, 2022 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants