[ios][contacts] Migrate to Expo Modules #25696
Conversation
expo-bot
added
the
bot: needs changes
|
Very excited for this one! It will definitely help building this: #2645 🔥 |
alanjhughes
force-pushed
the
@alanhughes/ios/contacts
branch
from
b4d3296
to
2992dbd
Compare
alanjhughes
force-pushed
the
@alanhughes/ios/contacts
branch
from
2992dbd
to
7dff4be
Compare
alanjhughes
force-pushed
the
@alanhughes/ios/contacts
branch
from
7dff4be
to
f683686
Compare
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇 ❌ Error: SwiftLint Violations
Generated by ExpoBot 🤖 against f683686 |
| private let delegate = ContactControllerDelegate() | ||
| private var presentingViewController: UIViewController? | ||
|
|
||
| public func definition() -> ModuleDefinition { |
There was a problem hiding this comment.
🔴 SwiftLint: Cyclomatic Complexity Violation
Function should have complexity 20 or less; currently complexity is 33
| } | ||
| }.runOnQueue(.main) | ||
|
|
||
| AsyncFunction("presentFormAsync") { (identifier: String?, data: Contact, options: FormOptions, promise: Promise) in |
There was a problem hiding this comment.
🔴 SwiftLint: Closure Body Length Violation
Closure body should span 50 lines or less excluding comments and whitespace: currently spans 53 lines
| } | ||
| } | ||
|
|
||
| private func mutateContact(_ contact: inout CNMutableContact, with data: Contact) throws { |
There was a problem hiding this comment.
🟠 SwiftLint: Cyclomatic Complexity Violation
Function should have complexity 20 or less; currently complexity is 26
| return descriptors | ||
| } | ||
|
|
||
| func serializeContact(person: CNContact, keys: [String]?, directory: URL?) throws -> [String: Any] { |
There was a problem hiding this comment.
🔴 SwiftLint: Cyclomatic Complexity Violation
Function should have complexity 20 or less; currently complexity is 37
|
Hey @alanjhughes , is there anything blocking this from getting merged? |
|
Hey @fobos531, no, it just needs to be reviewed. |
Okay, that is good to hear. If I want to contribute something to expo-contacts (both for iOS and Android) at this point, should I fork from your branch (@alanhughes/ios/contacts) or wait until this gets merged? |
| @@ -647,7 +647,7 @@ export async function getContactByIdAsync( | |||
| pageSize: 1, | |||
| pageOffset: 0, | |||
| fields, | |||
| id, | |||
| id: Array.isArray(id) ? id : [id], | |||
There was a problem hiding this comment.
This is probably not necessary, afaik if you have an array type on the native side, a single item will be automatically arrayized during conversion 😉
There was a problem hiding this comment.
It may not work on Android 😅 I don't remember if I implemented it.
Why
Closes ENG-10691
One of the last modules to be migrate
How
Usual migration steps.
Test Plan
bare-expo and NCL