Use LLDB and devicectl for deploying and debugging apps on iOS 17+ #173360
Conversation
github-actions
bot
added
platform-ios
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-structured refactoring for deploying and debugging apps on iOS 17+ devices (CoreDevices) by leveraging devicectl and lldb. The new IOSCoreDeviceLauncher and LLDB classes effectively encapsulate complex interactions, improving modularity and maintainability. The changes also include a fallback mechanism to the existing Xcode automation flow, which adds resilience. Overall, this is a great improvement. I have a couple of minor suggestions to improve code clarity and consistency.
| /// Pattern of lldb log when the process is stopped. | ||
| /// | ||
| /// Example: (lldb) Process 6152 stopped | ||
| static final _lldbProcessStopped = RegExp(r'Process \d* stopped'); |
There was a problem hiding this comment.
The _lldbProcessStopped regex uses \d*, which matches zero or more digits. This is inconsistent with _lldbProcessResuming on line 42, which uses \d+. If a process ID is always expected in the 'stopped' message, using \d+ would be more robust and consistent.
static final _lldbProcessStopped = RegExp(r'Process \d+ stopped');
| factory IOSCoreDeviceRunningProcess.fromJson(Map<String, Object?> data) { | ||
| return IOSCoreDeviceRunningProcess._( | ||
| executable: data['executable']?.toString(), | ||
| processIdentifier: data['processIdentifier'] is int? | ||
| ? data['processIdentifier'] as int? | ||
| : null, | ||
| ); | ||
| } |
There was a problem hiding this comment.
For clarity and improved readability, this casting logic can be simplified. Using a local variable and a type check makes the intent clearer, which aligns with the Flutter Style Guide's principle of optimizing for readability.1
factory IOSCoreDeviceRunningProcess.fromJson(Map<String, Object?> data) {
final Object? processId = data['processIdentifier'];
return IOSCoreDeviceRunningProcess._(
executable: data['executable']?.toString(),
processIdentifier: processId is int ? processId : null,
);
}
Style Guide References
Footnotes
|
This change needs to be cherry-picked into beta/stable. To make that easier, the plan is to split this into 3 PRs. 1 - Adds new classes and methods but will not be called or used yet. This will be large, but safe to cherry-pick since the new code will not be used. |
1 participant
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.