[flutter_tools] xcresult parser. #92604
Conversation
flutter-dashboard
bot
added
the
tool
packages/flutter_tools/test/general.shard/ios/xcresult_test.dart
Outdated
Show resolved
Hide resolved
| _kWhichSysctlCommand, | ||
| _kx64CheckCommand, | ||
| ]); | ||
| final Xcode xcode = Xcode.test( |
There was a problem hiding this comment.
This looks pretty squished, do you have your line lengths set to 120?
There was a problem hiding this comment.
It was 80, should it be 120?
There was a problem hiding this comment.
@christopherfujino Jonah and I both had it set to 120 but it's not enforced anywhere. What would you prefer here?
There was a problem hiding this comment.
The style guide says (which I personally try to follow):
Aim for a maximum line length of roughly 80 characters, but prefer going over if breaking the line would make it less readable, or if it would make the line less consistent with other nearby lines. Prefer avoiding line breaks after assignment operators.
There was a problem hiding this comment.
I remember framework repo doesn't require (and doesn't recommend) to use the dart formatter as sometimes the lines are sometimes too short to be readable. This is probably one of those cases. I will revert it back to 80 and do a manual scan to see if there are lines that are too short to be readable.
There was a problem hiding this comment.
Even with 80 it will look better now that you added all the trailing commas.
packages/flutter_tools/test/general.shard/ios/xcresult_test.dart
Outdated
Show resolved
Hide resolved
jmagman
added
the
t: xcode
jmagman
left a comment
jmagman
left a comment
There was a problem hiding this comment.
Tests are failing
01:05 +1864 ~4 -1: test\general.shard\ios\xcresult_test.dart: error: `xcresulttool get` process fail should return an `XCResult with `parsingErrorMessage` as stderr. [E]
Expected: 'Fake: fail to parse result json.'
Actual: 'xcresult parser: Unrecognized top level json format.'
Which: is different.
Expected: Fake: fail ...
Actual: xcresult p ...
^
Differ at offset 0
package:test_api expect
test\general.shard\ios\xcresult_test.dart 127:5 main.<fn>
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:convert' show json; |
There was a problem hiding this comment.
This is a forbidden import, instead use src/convert.dart
04:19 +18 -1: test\integration.shard\forbidden_imports_test.dart: no unauthorized imports of dart:convert [E]
lib\src\ios\xcresult.dart imports 'dart:convert'; import 'lib/src/convert.dart' instead
package:test_api fail
test\integration.shard\forbidden_imports_test.dart 207:13 main.<fn>
test\src\common.dart 165:18 test.<fn>
test\src\common.dart 161:5 test.<fn
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:convert' show json; |
| /// The type of the issue. | ||
| /// | ||
| /// The possible values are `warning`, `error` etc. | ||
| final String type; |
There was a problem hiding this comment.
yup, done.
I made the enum named type, and added a subType field as String
jmagman
left a comment
jmagman
left a comment
There was a problem hiding this comment.
Small nit, thanks for the edits, LGTM!
| /// `issueJson` is the object at xcresultJson[['actions']['_values'][0]['buildResult']['issues']['errorSummaries'/'warningSummaries']['_values']. | ||
| factory XCResultIssue( | ||
| {required XCResultIssueType type, | ||
| required Map<String, Object?> issueJson}) { |
4 participants
Add
XCResultGenerator, this object generates anXCResult, which contains useful information in an iOS or macOS build.As this initial version.
XCResultonly contains issues such as warning and error messages.Part of #22536
After this lands, the build_ios.dart can adopt this and show the warning and error messages to the console.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.