Repository navigation
Add a user friendly error message when no development team is selected for physical deployment - #7879
Conversation
This reverts commit 36ed0c0.
|
For #7839 |
| allowReentrantFlutter: true | ||
| ); | ||
| if (checkBuildSettings.exitCode == 0 | ||
| && !checkBuildSettings.stdout?.contains('DEVELOPMENT_TEAM') == true) { |
There was a problem hiding this comment.
This is definitely the case with recent releases of Xcode (> 8.1, IIRC) when using automatic signing -- is it also the case in Xcode 7 and/or when you use manual mode and use a provisioning profile?
One option would be to bump the minimum required Xcode version from its current value (IIRC, >= 7.0) in mac.dart.
There was a problem hiding this comment.
Check both development team and provisioning profile as suggested
| printError(' https://flutter.io/setup/#deploy-to-ios-devices\n'); | ||
| printError('Or run on an iOS simulator'); | ||
| printError( | ||
| '═══════════════════════════════════════════════════════════════════════════════════'); |
There was a problem hiding this comment.
Consider using a multi-line ''' string.
There was a problem hiding this comment.
yeah that might be more readable.
| if (result.buildForDevice | ||
| && result.buildCommands != null | ||
| && result.buildCommands.contains('build') | ||
| && result.appDirectory != null) { |
There was a problem hiding this comment.
When buildForDevice is true, are buildCommands and appDirectory always expected? If so, use assertions ahead of the conditional.
There was a problem hiding this comment.
Some grouping would indeed be nicer. Pulled out an inner class and used assertions.
| printError('Encountered error while building for $logTarget.'); | ||
| diagnoseXcodeBuildFailure(result); | ||
| await diagnoseXcodeBuildFailure(result); | ||
| throwToolExit(''); |
There was a problem hiding this comment.
It seems hacky to call it with an empty string.
There was a problem hiding this comment.
Swapped in the message 2 lines earlier. Makes sense to print the definitive implication at the end anyway.
| printStatus(result.stdout); | ||
| return new XcodeBuildResult(false, stdout: result.stdout, stderr: result.stderr); | ||
| return new XcodeBuildResult( | ||
| false, |
There was a problem hiding this comment.
we should use named arguments for all boolean parameters.
| return; | ||
| } | ||
| if (result.buildForDevice | ||
| && result.buildCommands != null |
There was a problem hiding this comment.
if you put the && at the end of the lines you'll get the subexpressions lined up
| if (checkBuildSettings.exitCode == 0 | ||
| && !checkBuildSettings.stdout?.contains('DEVELOPMENT_TEAM') == true) { | ||
| printError( | ||
| '═══════════════════════════════════════════════════════════════════════════════════'); |
There was a problem hiding this comment.
I don't think wrapping here is helping readability.
| this.stderr, | ||
| this.buildCommands, | ||
| this.buildForDevice, | ||
| this.appDirectory |
|
LGTM but I don't really know much about Xcode. |
4 participants
No description provided.