Unverified Commit 060e4608 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

clean up stale or unnecessary TODOS (#88308)

parent d39d5426
...@@ -21,7 +21,6 @@ const String kFirstRecompileTime = 'FirstRecompileTime'; ...@@ -21,7 +21,6 @@ const String kFirstRecompileTime = 'FirstRecompileTime';
const String kSecondStartupTime = 'SecondStartupTime'; const String kSecondStartupTime = 'SecondStartupTime';
const String kSecondRestartTime = 'SecondRestartTime'; const String kSecondRestartTime = 'SecondRestartTime';
abstract class WebDevice { abstract class WebDevice {
static const String chrome = 'chrome'; static const String chrome = 'chrome';
static const String webServer = 'web-server'; static const String webServer = 'web-server';
...@@ -61,7 +60,8 @@ TaskFunction createWebDevModeTest(String webDevice, bool enableIncrementalCompil ...@@ -61,7 +60,8 @@ TaskFunction createWebDevModeTest(String webDevice, bool enableIncrementalCompil
.transform<String>(utf8.decoder) .transform<String>(utf8.decoder)
.transform<String>(const LineSplitter()) .transform<String>(const LineSplitter())
.listen((String line) { .listen((String line) {
// TODO(jonahwilliams): non-dwds builds do not know when the browser is loaded. // non-dwds builds do not know when the browser is loaded so keep trying
// until this succeeds.
if (line.contains('Ignoring terminal input')) { if (line.contains('Ignoring terminal input')) {
Future<void>.delayed(const Duration(seconds: 1)).then((void _) { Future<void>.delayed(const Duration(seconds: 1)).then((void _) {
process.stdin.write(restarted ? 'q' : 'r'); process.stdin.write(restarted ? 'q' : 'r');
...@@ -139,7 +139,8 @@ TaskFunction createWebDevModeTest(String webDevice, bool enableIncrementalCompil ...@@ -139,7 +139,8 @@ TaskFunction createWebDevModeTest(String webDevice, bool enableIncrementalCompil
.transform<String>(utf8.decoder) .transform<String>(utf8.decoder)
.transform<String>(const LineSplitter()) .transform<String>(const LineSplitter())
.listen((String line) { .listen((String line) {
// TODO(jonahwilliams): non-dwds builds do not know when the browser is loaded. // non-dwds builds do not know when the browser is loaded so keep trying
// until this succeeds.
if (line.contains('Ignoring terminal input')) { if (line.contains('Ignoring terminal input')) {
Future<void>.delayed(const Duration(seconds: 1)).then((void _) { Future<void>.delayed(const Duration(seconds: 1)).then((void _) {
process.stdin.write(restarted ? 'q' : 'r'); process.stdin.write(restarted ? 'q' : 'r');
......
...@@ -572,7 +572,9 @@ class AnimationController extends Animation<double> ...@@ -572,7 +572,9 @@ class AnimationController extends Animation<double>
case AnimationBehavior.normal: case AnimationBehavior.normal:
// Since the framework cannot handle zero duration animations, we run it at 5% of the normal // Since the framework cannot handle zero duration animations, we run it at 5% of the normal
// duration to limit most animations to a single frame. // duration to limit most animations to a single frame.
// TODO(jonahwilliams): determine a better process for setting duration. // Ideally, the framework would be able to handle zero duration animations, however, the common
// pattern of an eternally repeating animation might cause an endless loop if it weren't delayed
// for at least one frame.
scale = 0.05; scale = 0.05;
break; break;
case AnimationBehavior.preserve: case AnimationBehavior.preserve:
......
...@@ -143,7 +143,7 @@ mixin LocalComparisonOutput { ...@@ -143,7 +143,7 @@ mixin LocalComparisonOutput {
String additionalFeedback = ''; String additionalFeedback = '';
if (result.diffs != null) { if (result.diffs != null) {
additionalFeedback = '\nFailure feedback can be found at ${path.join(basedir.path, 'failures')}'; additionalFeedback = '\nFailure feedback can be found at ${path.join(basedir.path, 'failures')}';
final Map<String, Image> diffs = result.diffs!.cast<String, Image>(); final Map<String, Image> diffs = result.diffs!;
for (final MapEntry<String, Image> entry in diffs.entries) { for (final MapEntry<String, Image> entry in diffs.entries) {
final File output = getFailureFile( final File output = getFailureFile(
key.isEmpty ? entry.key : '${entry.key}_$key', key.isEmpty ? entry.key : '${entry.key}_$key',
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:typed_data'; import 'dart:typed_data';
import 'dart:ui';
import 'package:path/path.dart' as path; import 'package:path/path.dart' as path;
import '_goldens_io.dart' if (dart.library.html) '_goldens_web.dart' as _goldens; import '_goldens_io.dart' if (dart.library.html) '_goldens_web.dart' as _goldens;
...@@ -326,8 +327,7 @@ class ComparisonResult { ...@@ -326,8 +327,7 @@ class ComparisonResult {
/// Map containing differential images to illustrate found variants in pixel /// Map containing differential images to illustrate found variants in pixel
/// values in the execution of the pixel test. /// values in the execution of the pixel test.
// TODO(jonahwilliams): fix type signature when image is updated to support web. final Map<String, Image>? diffs;
final Map<String, Object>? diffs;
/// The calculated percentage of pixel difference between two images. /// The calculated percentage of pixel difference between two images.
final double diffPercent; final double diffPercent;
......
...@@ -55,7 +55,7 @@ Future<void> main(List<String> args) async { ...@@ -55,7 +55,7 @@ Future<void> main(List<String> args) async {
final String packages = '$buildDirectory/dartlang/gen/$path/${name}_dart_library.packages'; final String packages = '$buildDirectory/dartlang/gen/$path/${name}_dart_library.packages';
final String outputDill = '$buildDirectory/${name}_tmp.dill'; final String outputDill = '$buildDirectory/${name}_tmp.dill';
// TODO(jonahwilliams): running from fuchsia root hangs hot reload for some reason. // Running from fuchsia root hangs hot reload for some reason.
// switch to the project root directory and run from there. // switch to the project root directory and run from there.
originalWorkingDirectory = globals.fs.currentDirectory.path; originalWorkingDirectory = globals.fs.currentDirectory.path;
globals.fs.currentDirectory = path; globals.fs.currentDirectory = path;
...@@ -92,7 +92,7 @@ Future<void> main(List<String> args) async { ...@@ -92,7 +92,7 @@ Future<void> main(List<String> args) async {
'--target', '--target',
targetFile, targetFile,
'--target-model', '--target-model',
'flutter', // TODO(jonahwilliams): change to flutter_runner when dart SDK rolls 'flutter_runner',
'--output-dill', '--output-dill',
outputDill, outputDill,
'--packages', '--packages',
......
...@@ -605,7 +605,7 @@ class FlutterBuildSystem extends BuildSystem { ...@@ -605,7 +605,7 @@ class FlutterBuildSystem extends BuildSystem {
// Always persist the file cache to disk. // Always persist the file cache to disk.
fileCache.persist(); fileCache.persist();
} }
// TODO(jonahwilliams): this is a bit of a hack, due to various parts of // This is a bit of a hack, due to various parts of
// the flutter tool writing these files unconditionally. Since Xcode uses // the flutter tool writing these files unconditionally. Since Xcode uses
// timestamps to track files, this leads to unnecessary rebuilds if they // timestamps to track files, this leads to unnecessary rebuilds if they
// are included. Once all the places that write these files have been // are included. Once all the places that write these files have been
......
...@@ -23,9 +23,6 @@ class GenerateLocalizationsTarget extends Target { ...@@ -23,9 +23,6 @@ class GenerateLocalizationsTarget extends Target {
List<Source> get inputs => <Source>[ List<Source> get inputs => <Source>[
// This is added as a convenience for developing the tool. // This is added as a convenience for developing the tool.
const Source.pattern('{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/targets/localizations.dart'), const Source.pattern('{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/targets/localizations.dart'),
// TODO(jonahwilliams): once https://github.com/flutter/flutter/issues/56321 is
// complete, we should add the artifact as a dependency here. Since the tool runs
// this code from source, looking up each dependency will be cumbersome.
]; ];
@override @override
......
...@@ -371,9 +371,7 @@ abstract class CreateBase extends FlutterCommand { ...@@ -371,9 +371,7 @@ abstract class CreateBase extends FlutterCommand {
'pluginClassSnakeCase': pluginClassSnakeCase, 'pluginClassSnakeCase': pluginClassSnakeCase,
'pluginClassCapitalSnakeCase': pluginClassCapitalSnakeCase, 'pluginClassCapitalSnakeCase': pluginClassCapitalSnakeCase,
'pluginDartClass': pluginDartClass, 'pluginDartClass': pluginDartClass,
// TODO(jonahwilliams): update after google3 uuid is updated. 'pluginProjectUUID': const Uuid().v4().toUpperCase(),
// ignore: prefer_const_constructors
'pluginProjectUUID': Uuid().v4().toUpperCase(),
'withPluginHook': withPluginHook, 'withPluginHook': withPluginHook,
'androidLanguage': androidLanguage, 'androidLanguage': androidLanguage,
'iosLanguage': iosLanguage, 'iosLanguage': iosLanguage,
......
...@@ -740,9 +740,6 @@ class FuchsiaDevice extends Device { ...@@ -740,9 +740,6 @@ class FuchsiaDevice extends Device {
/// provided set of `ports`. /// provided set of `ports`.
/// ///
/// Returns null if no isolate port can be found. /// Returns null if no isolate port can be found.
///
// TODO(jonahwilliams): replacing this with the hub will require an update
// to the flutter_runner.
Future<int> findIsolatePort(String isolateName, List<int> ports) async { Future<int> findIsolatePort(String isolateName, List<int> ports) async {
for (final int port in ports) { for (final int port in ports) {
try { try {
......
...@@ -58,7 +58,6 @@ class MDnsObservatoryDiscovery { ...@@ -58,7 +58,6 @@ class MDnsObservatoryDiscovery {
/// If it is null and there is only one available instance of Observatory, /// If it is null and there is only one available instance of Observatory,
/// it will return that instance's information regardless of what application /// it will return that instance's information regardless of what application
/// the Observatory instance is for. /// the Observatory instance is for.
// TODO(jonahwilliams): use `deviceVmservicePort` to filter mdns results.
@visibleForTesting @visibleForTesting
Future<MDnsObservatoryDiscoveryResult> query({String applicationId, int deviceVmservicePort}) async { Future<MDnsObservatoryDiscoveryResult> query({String applicationId, int deviceVmservicePort}) async {
_logger.printTrace('Checking for advertised Dart observatories...'); _logger.printTrace('Checking for advertised Dart observatories...');
......
...@@ -155,8 +155,6 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner { ...@@ -155,8 +155,6 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner {
testWrapper.registerPlatformPlugin( testWrapper.registerPlatformPlugin(
<Runtime>[Runtime.chrome], <Runtime>[Runtime.chrome],
() { () {
// TODO(jonahwilliams): refactor this into a factory that handles
// providing dependencies.
return FlutterWebPlatform.start( return FlutterWebPlatform.start(
flutterProject.directory.path, flutterProject.directory.path,
updateGoldens: updateGoldens, updateGoldens: updateGoldens,
......
...@@ -54,9 +54,6 @@ class FlutterTesterDevice extends Device { ...@@ -54,9 +54,6 @@ class FlutterTesterDevice extends Device {
@required ProcessManager processManager, @required ProcessManager processManager,
@required FlutterVersion flutterVersion, @required FlutterVersion flutterVersion,
@required Logger logger, @required Logger logger,
// TODO(jonahwilliams): remove once g3 rolls.
// ignore: avoid_unused_constructor_parameters
String buildDirectory,
@required FileSystem fileSystem, @required FileSystem fileSystem,
@required Artifacts artifacts, @required Artifacts artifacts,
@required OperatingSystemUtils operatingSystemUtils, @required OperatingSystemUtils operatingSystemUtils,
......
...@@ -129,7 +129,6 @@ String generateMainModule({ ...@@ -129,7 +129,6 @@ String generateMainModule({
required bool nativeNullAssertions, required bool nativeNullAssertions,
String bootstrapModule = 'main_module.bootstrap', String bootstrapModule = 'main_module.bootstrap',
}) { }) {
// TODO(jonahwilliams): fix typo in dwds and update.
return ''' return '''
/* ENTRYPOINT_EXTENTION_MARKER */ /* ENTRYPOINT_EXTENTION_MARKER */
// Create the main module loaded below. // Create the main module loaded below.
......
...@@ -57,8 +57,8 @@ abstract class ChromiumDevice extends Device { ...@@ -57,8 +57,8 @@ abstract class ChromiumDevice extends Device {
/// The active chrome instance. /// The active chrome instance.
Chromium _chrome; Chromium _chrome;
// TODO(jonahwilliams): this is technically false, but requires some refactoring // This device does not actually support hot reload, but the current implementation of the resident runner
// to allow hot mode restart only devices. // requires both supportsHotReload and supportsHotRestart to be true in order to allow hot restart.
@override @override
bool get supportsHotReload => true; bool get supportsHotReload => true;
......
...@@ -96,7 +96,6 @@ void main() { ...@@ -96,7 +96,6 @@ void main() {
fileSystem: fileSystem, fileSystem: fileSystem,
processManager: fakeProcessManager, processManager: fakeProcessManager,
artifacts: Artifacts.test(), artifacts: Artifacts.test(),
buildDirectory: 'build',
logger: BufferLogger.test(), logger: BufferLogger.test(),
flutterVersion: FakeFlutterVersion(), flutterVersion: FakeFlutterVersion(),
operatingSystemUtils: FakeOperatingSystemUtils(), operatingSystemUtils: FakeOperatingSystemUtils(),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment