Feature/adjust patrol to add2app#3037
Conversation
…config-only` in add-to-app mode, ensuring correct Dart entrypoint configuration for Patrol test bundles.
github-actions
Bot
added
the
package: patrol_cli
![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 support for Flutter "add-to-app" (module) projects within the Patrol CLI. It adds new CLI flags and configuration options in pubspec.yaml to specify external native project paths for Android and iOS. The build logic for both platforms has been updated to resolve working directories dynamically and ensure that configuration-only builds are executed correctly from the module root. Feedback focuses on improving the robustness of the Gradle wrapper detection on Windows, removing redundant type filtering during directory listing, and cleaning up idiomatic type checks in the pubspec reader.
| final gradlew = p.join(resolvedNativePath, 'gradlew'); | ||
| if (!FileSystemEntity.isFileSync(gradlew)) { | ||
| throw Exception( | ||
| 'No gradlew script found in $resolvedNativePath. ' | ||
| 'The native Android project must have a Gradle wrapper.', | ||
| ); | ||
| } |
There was a problem hiding this comment.
On Windows, the Gradle wrapper script is named gradlew.bat. Checking only for gradlew might lead to false negatives if the extensionless shell script is missing, even though standard Flutter projects usually include both. It is safer to check for either.
final gradlew = p.join(resolvedNativePath, 'gradlew');
final gradlewBat = p.join(resolvedNativePath, 'gradlew.bat');
if (!FileSystemEntity.isFileSync(gradlew) &&
!FileSystemEntity.isFileSync(gradlewBat)) {
throw Exception(
'No gradlew or gradlew.bat script found in $resolvedNativePath. '
'The native Android project must have a Gradle wrapper.',
);
}|
|
||
| final workspaces = dir | ||
| .listSync() | ||
| .whereType<FileSystemEntity>() |
There was a problem hiding this comment.
The call to whereType<FileSystemEntity>() is redundant here because listSync() already returns a List<FileSystemEntity>.
| .whereType<FileSystemEntity>() | |
| .listSync() |
| androidConfig.flavor = flavor; | ||
| } | ||
| final dynamic nativeProjectPath = android['native_project_path']; | ||
| if (nativeProjectPath != null && nativeProjectPath is String?) { |
There was a problem hiding this comment.
The type check is String? is redundant when combined with a null check. Since nativeProjectPath != null is already verified, using is String is more precise and idiomatic.
| if (nativeProjectPath != null && nativeProjectPath is String?) { | |
| if (nativeProjectPath != null && nativeProjectPath is String) { |
| iosConfig.flavor = flavor; | ||
| } | ||
| final dynamic nativeProjectPath = ios['native_project_path']; | ||
| if (nativeProjectPath != null && nativeProjectPath is String?) { |
There was a problem hiding this comment.
The type check is String? is redundant when combined with a null check. Since nativeProjectPath != null is already verified, using is String is more precise and idiomatic.
| if (nativeProjectPath != null && nativeProjectPath is String?) { | |
| if (nativeProjectPath != null && nativeProjectPath is String) { |
…le.properties, enhancing support for custom Android project structures in add-to-app setups.
1 participant
No description provided.