-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Removes ios universal link vmservices and let xcodeproject to dump js… #133709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
vashworth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that your new method outputUniversalLinkSettings is not called anywhere (other than tests). Is that intentional?
| final File outputFile = fs.file(outputFilePath); | ||
| final Map<String, Object?> json = jsonDecode(outputFile.readAsStringSync()) as Map<String, Object?>; | ||
| expect(json['teamIdentifier'], 'ABC'); | ||
| expect(json['bundleIdentifier'], 'io.flutter.someProject'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a expectation for associatedDomains here? I believe you'd expect it to be empty list?
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:convert'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error from failing tests:
lib/src/xcode_project.dart imports 'dart:convert'; import 'lib/src/convert.dart' instead
There will be another pr that wire up outputUniversalLinkSettings to flutter analyze command |
vashworth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you also need to update integration.shard/vmservice_integration_test.dart
vashworth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…on file
The deeplink validation tool will become an static app so it can't no longer access vm services.
The goal will be then to turn them into flutter analyze command similar to
flutter analyze --android --[options]that static app can use on.This pr only removes vm services and turn the api to dump a output file instead of printing everything to stdout.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.