Unverified Commit b7261586 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Audit `TODO` syntax (#20837)

Fixes the pattern for some TODOs to match our style guide.

(Also, a couple of minor code order fixes.)
parent adafaee0
...@@ -26,7 +26,7 @@ analyzer: ...@@ -26,7 +26,7 @@ analyzer:
errors: errors:
# treat missing required parameters as a warning (not a hint) # treat missing required parameters as a warning (not a hint)
missing_required_param: warning missing_required_param: warning
# TODO: https://github.com/flutter/flutter/issues/20114 # TODO(devoncarew): https://github.com/flutter/flutter/issues/20114
# treat missing returns as a warning (not a hint) # treat missing returns as a warning (not a hint)
missing_return: ignore missing_return: ignore
# allow having TODOs in the code # allow having TODOs in the code
......
...@@ -399,7 +399,7 @@ class IosDeviceDiscovery implements DeviceDiscovery { ...@@ -399,7 +399,7 @@ class IosDeviceDiscovery implements DeviceDiscovery {
Future<Map<String, HealthCheckResult>> checkDevices() async { Future<Map<String, HealthCheckResult>> checkDevices() async {
final Map<String, HealthCheckResult> results = <String, HealthCheckResult>{}; final Map<String, HealthCheckResult> results = <String, HealthCheckResult>{};
for (String deviceId in await discoverDevices()) { for (String deviceId in await discoverDevices()) {
// TODO: do a more meaningful connectivity check than just recording the ID // TODO(ianh): do a more meaningful connectivity check than just recording the ID
results['ios-device-$deviceId'] = new HealthCheckResult.success(); results['ios-device-$deviceId'] = new HealthCheckResult.success();
} }
return results; return results;
......
...@@ -9,7 +9,7 @@ import 'keys.dart' as keys; ...@@ -9,7 +9,7 @@ import 'keys.dart' as keys;
void main() { void main() {
enableFlutterDriverExtension(handler: (String message) async { enableFlutterDriverExtension(handler: (String message) async {
// TODO(cbernaschina) remove when test flakiness is resolved // TODO(cbernaschina): remove when test flakiness is resolved
return 'keyboard_resize'; return 'keyboard_resize';
}); });
runApp(new MyApp()); runApp(new MyApp());
......
...@@ -911,7 +911,7 @@ class _PressableActionButtonState extends State<_PressableActionButton> { ...@@ -911,7 +911,7 @@ class _PressableActionButtonState extends State<_PressableActionButton> {
Widget build(BuildContext context) { Widget build(BuildContext context) {
return new _ActionButtonParentDataWidget( return new _ActionButtonParentDataWidget(
isPressed: _isPressed, isPressed: _isPressed,
// TODO:(mattcarroll): Button press dynamics need overhaul for iOS: https://github.com/flutter/flutter/issues/19786 // TODO(mattcarroll): Button press dynamics need overhaul for iOS: https://github.com/flutter/flutter/issues/19786
child: new GestureDetector( child: new GestureDetector(
excludeFromSemantics: true, excludeFromSemantics: true,
behavior: HitTestBehavior.opaque, behavior: HitTestBehavior.opaque,
......
...@@ -51,7 +51,7 @@ class _ToolbarContainerLayout extends SingleChildLayoutDelegate { ...@@ -51,7 +51,7 @@ class _ToolbarContainerLayout extends SingleChildLayoutDelegate {
bool shouldRelayout(_ToolbarContainerLayout oldDelegate) => false; bool shouldRelayout(_ToolbarContainerLayout oldDelegate) => false;
} }
// TODO(eseidel) Toolbar needs to change size based on orientation: // TODO(eseidel): Toolbar needs to change size based on orientation:
// http://material.google.com/layout/structure.html#structure-app-bar // http://material.google.com/layout/structure.html#structure-app-bar
// Mobile Landscape: 48dp // Mobile Landscape: 48dp
// Mobile Portrait: 56dp // Mobile Portrait: 56dp
......
...@@ -962,7 +962,7 @@ class _RenderSlider extends RenderBox { ...@@ -962,7 +962,7 @@ class _RenderSlider extends RenderBox {
void _paintOverlay(Canvas canvas, Offset center) { void _paintOverlay(Canvas canvas, Offset center) {
if (!_overlayAnimation.isDismissed) { if (!_overlayAnimation.isDismissed) {
// TODO(gspencer) : We don't really follow the spec here for overlays. // TODO(gspencer): We don't really follow the spec here for overlays.
// The spec says to use 16% opacity for drawing over light material, // The spec says to use 16% opacity for drawing over light material,
// and 32% for colored material, but we don't really have a way to // and 32% for colored material, but we don't really have a way to
// know what the underlying color is, so there's no easy way to // know what the underlying color is, so there's no easy way to
......
...@@ -889,7 +889,7 @@ class _NestedScrollController extends ScrollController { ...@@ -889,7 +889,7 @@ class _NestedScrollController extends ScrollController {
} }
Iterable<_NestedScrollPosition> get nestedPositions sync* { Iterable<_NestedScrollPosition> get nestedPositions sync* {
// TODO(vegorov) use instance method version of castFrom when it is available. // TODO(vegorov): use instance method version of castFrom when it is available.
yield* Iterable.castFrom<ScrollPosition, _NestedScrollPosition>(positions); yield* Iterable.castFrom<ScrollPosition, _NestedScrollPosition>(positions);
} }
} }
......
...@@ -812,8 +812,9 @@ class WidgetInspectorService { ...@@ -812,8 +812,9 @@ class WidgetInspectorService {
/// the value is returned over the Observatory protocol and when the /// the value is returned over the Observatory protocol and when the
/// separate observatory protocol command has to be used to retrieve its full /// separate observatory protocol command has to be used to retrieve its full
/// contents. /// contents.
/// TODO(jacobr): Replace this with a better solution once //
/// https://github.com/dart-lang/sdk/issues/32919 is fixed. // TODO(jacobr): Replace this with a better solution once
// https://github.com/dart-lang/sdk/issues/32919 is fixed.
String _safeJsonEncode(Object object) { String _safeJsonEncode(Object object) {
final String jsonString = json.encode(object); final String jsonString = json.encode(object);
_serializeRing[_serializeRingIndex] = jsonString; _serializeRing[_serializeRingIndex] = jsonString;
......
...@@ -6,8 +6,8 @@ import 'package:flutter/foundation.dart'; ...@@ -6,8 +6,8 @@ import 'package:flutter/foundation.dart';
import '../flutter_test_alternative.dart'; import '../flutter_test_alternative.dart';
void main() { void main() {
// TODO(8128): These tests and the filtering mechanism should be revisited to account for causal async stack traces. // TODO(ianh): These tests and the filtering mechanism should be revisited to
// account for causal async stack traces. https://github.com/flutter/flutter/issues/8128
test('FlutterError.defaultStackFilter', () { test('FlutterError.defaultStackFilter', () {
final List<String> filtered = FlutterError.defaultStackFilter(StackTrace.current.toString().trimRight().split('\n')).toList(); final List<String> filtered = FlutterError.defaultStackFilter(StackTrace.current.toString().trimRight().split('\n')).toList();
expect(filtered.length, greaterThanOrEqualTo(4)); expect(filtered.length, greaterThanOrEqualTo(4));
......
...@@ -136,11 +136,10 @@ void main() { ...@@ -136,11 +136,10 @@ void main() {
expect(tester.getSize(find.byType(FlatButton)), equals(const Size(88.0, 48.0))); expect(tester.getSize(find.byType(FlatButton)), equals(const Size(88.0, 48.0)));
// Scaled text rendering is different on Linux and Mac by one pixel. // Scaled text rendering is different on Linux and Mac by one pixel.
// TODO(#12357): Update this test when text rendering is fixed. // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357
expect(tester.getSize(find.byType(Text)).width, isIn(<double>[54.0, 55.0])); expect(tester.getSize(find.byType(Text)).width, isIn(<double>[54.0, 55.0]));
expect(tester.getSize(find.byType(Text)).height, isIn(<double>[18.0, 19.0])); expect(tester.getSize(find.byType(Text)).height, isIn(<double>[18.0, 19.0]));
// Set text scale large enough to expand text and button. // Set text scale large enough to expand text and button.
await tester.pumpWidget( await tester.pumpWidget(
new Directionality( new Directionality(
...@@ -160,7 +159,7 @@ void main() { ...@@ -160,7 +159,7 @@ void main() {
); );
// Scaled text rendering is different on Linux and Mac by one pixel. // Scaled text rendering is different on Linux and Mac by one pixel.
// TODO(#12357): Update this test when text rendering is fixed. // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357
expect(tester.getSize(find.byType(FlatButton)).width, isIn(<double>[158.0, 159.0])); expect(tester.getSize(find.byType(FlatButton)).width, isIn(<double>[158.0, 159.0]));
expect(tester.getSize(find.byType(FlatButton)).height, equals(48.0)); expect(tester.getSize(find.byType(FlatButton)).height, equals(48.0));
expect(tester.getSize(find.byType(Text)).width, isIn(<double>[126.0, 127.0])); expect(tester.getSize(find.byType(Text)).width, isIn(<double>[126.0, 127.0]));
......
...@@ -241,11 +241,10 @@ void main() { ...@@ -241,11 +241,10 @@ void main() {
expect(tester.getSize(find.byType(FlatButton)), equals(const Size(88.0, 48.0))); expect(tester.getSize(find.byType(FlatButton)), equals(const Size(88.0, 48.0)));
// Scaled text rendering is different on Linux and Mac by one pixel. // Scaled text rendering is different on Linux and Mac by one pixel.
// TODO(#12357): Update this test when text rendering is fixed. // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357
expect(tester.getSize(find.byType(Text)).width, isIn(<double>[54.0, 55.0])); expect(tester.getSize(find.byType(Text)).width, isIn(<double>[54.0, 55.0]));
expect(tester.getSize(find.byType(Text)).height, isIn(<double>[18.0, 19.0])); expect(tester.getSize(find.byType(Text)).height, isIn(<double>[18.0, 19.0]));
// Set text scale large enough to expand text and button. // Set text scale large enough to expand text and button.
await tester.pumpWidget( await tester.pumpWidget(
new Directionality( new Directionality(
...@@ -265,7 +264,7 @@ void main() { ...@@ -265,7 +264,7 @@ void main() {
); );
// Scaled text rendering is different on Linux and Mac by one pixel. // Scaled text rendering is different on Linux and Mac by one pixel.
// TODO(#12357): Update this test when text rendering is fixed. // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357
expect(tester.getSize(find.byType(FlatButton)).width, isIn(<double>[158.0, 159.0])); expect(tester.getSize(find.byType(FlatButton)).width, isIn(<double>[158.0, 159.0]));
expect(tester.getSize(find.byType(FlatButton)).height, equals(48.0)); expect(tester.getSize(find.byType(FlatButton)).height, equals(48.0));
expect(tester.getSize(find.byType(Text)).width, isIn(<double>[126.0, 127.0])); expect(tester.getSize(find.byType(Text)).width, isIn(<double>[126.0, 127.0]));
......
...@@ -255,8 +255,7 @@ void main() { ...@@ -255,8 +255,7 @@ void main() {
paragraph.layout(const BoxConstraints()); paragraph.layout(const BoxConstraints());
// anyOf is needed here because Linux and Mac have different text // anyOf is needed here because Linux and Mac have different text
// rendering widths in tests. // rendering widths in tests.
// TODO(#12357): Figure out why this is, and fix it (if needed) once Blink // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357
// text rendering is replaced.
expect(paragraph.size.width, anyOf(79.0, 78.0)); expect(paragraph.size.width, anyOf(79.0, 78.0));
expect(paragraph.size.height, 26.0); expect(paragraph.size.height, 26.0);
...@@ -272,8 +271,7 @@ void main() { ...@@ -272,8 +271,7 @@ void main() {
// anyOf is needed here and below because Linux and Mac have different text // anyOf is needed here and below because Linux and Mac have different text
// rendering widths in tests. // rendering widths in tests.
// TODO(#12357): Figure out why this is, and fix it (if needed) once Blink // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357
// text rendering is replaced.
expect(boxes[0].toRect().width, anyOf(14.0, 13.0)); expect(boxes[0].toRect().width, anyOf(14.0, 13.0));
expect(boxes[0].toRect().height, closeTo(13.0, 0.0001)); expect(boxes[0].toRect().height, closeTo(13.0, 0.0001));
expect(boxes[1].toRect().width, anyOf(27.0, 26.0)); expect(boxes[1].toRect().width, anyOf(27.0, 26.0));
......
...@@ -127,7 +127,7 @@ List<TimelineEvent> _parseEvents(Map<String, dynamic> json) { ...@@ -127,7 +127,7 @@ List<TimelineEvent> _parseEvents(Map<String, dynamic> json) {
if (jsonEvents == null) if (jsonEvents == null)
return null; return null;
// TODO(vegorov) use instance method version of castFrom when it is available. // TODO(vegorov): use instance method version of castFrom when it is available.
return Iterable.castFrom<dynamic, Map<String, dynamic>>(jsonEvents) return Iterable.castFrom<dynamic, Map<String, dynamic>>(jsonEvents)
.map((Map<String, dynamic> eventJson) => new TimelineEvent(eventJson)) .map((Map<String, dynamic> eventJson) => new TimelineEvent(eventJson))
.toList(); .toList();
......
...@@ -395,10 +395,10 @@ abstract class GlobalMaterialLocalizations implements MaterialLocalizations { ...@@ -395,10 +395,10 @@ abstract class GlobalMaterialLocalizations implements MaterialLocalizations {
/// The script category used by [localTextGeometry]. Must be one of the strings /// The script category used by [localTextGeometry]. Must be one of the strings
/// declared in [MaterialTextGeometry]. /// declared in [MaterialTextGeometry].
/// //
/// TODO(ianh): make this return a TextTheme from MaterialTextGeometry. // TODO(ianh): make this return a TextTheme from MaterialTextGeometry.
/// TODO(ianh): drop the constructor on MaterialTextGeometry. // TODO(ianh): drop the constructor on MaterialTextGeometry.
/// TODO(ianh): drop the strings on MaterialTextGeometry. // TODO(ianh): drop the strings on MaterialTextGeometry.
@protected @protected
String get scriptCategory; String get scriptCategory;
......
...@@ -280,7 +280,7 @@ ThinAppFrameworks() { ...@@ -280,7 +280,7 @@ ThinAppFrameworks() {
# Main entry point. # Main entry point.
# TODO(cbracken) improve error handling, then enable set -e # TODO(cbracken): improve error handling, then enable set -e
if [[ $# == 0 ]]; then if [[ $# == 0 ]]; then
# Backwards-compatibility: if no args are provided, build. # Backwards-compatibility: if no args are provided, build.
......
...@@ -451,7 +451,7 @@ class AndroidDevice extends Device { ...@@ -451,7 +451,7 @@ class AndroidDevice extends Device {
// device has printed "Observatory is listening on...". // device has printed "Observatory is listening on...".
printTrace('Waiting for observatory port to be available...'); printTrace('Waiting for observatory port to be available...');
// TODO(danrubel) Waiting for observatory services can be made common across all devices. // TODO(danrubel): Waiting for observatory services can be made common across all devices.
try { try {
Uri observatoryUri; Uri observatoryUri;
......
...@@ -230,7 +230,7 @@ class AOTSnapshotter { ...@@ -230,7 +230,7 @@ class AOTSnapshotter {
if (platform == TargetPlatform.android_arm || iosArch == IOSArch.armv7) { if (platform == TargetPlatform.android_arm || iosArch == IOSArch.armv7) {
// Use softfp for Android armv7 devices. // Use softfp for Android armv7 devices.
// Note that this is the default for armv7 iOS builds, but harmless to set. // Note that this is the default for armv7 iOS builds, but harmless to set.
// TODO(cbracken) eliminate this when we fix https://github.com/flutter/flutter/issues/17489 // TODO(cbracken): eliminate this when we fix https://github.com/flutter/flutter/issues/17489
genSnapshotArgs.add('--no-sim-use-hardfp'); genSnapshotArgs.add('--no-sim-use-hardfp');
// Not supported by the Pixel in 32-bit mode. // Not supported by the Pixel in 32-bit mode.
...@@ -469,7 +469,7 @@ class CoreJITSnapshotter { ...@@ -469,7 +469,7 @@ class CoreJITSnapshotter {
if (platform == TargetPlatform.android_arm) { if (platform == TargetPlatform.android_arm) {
// Use softfp for Android armv7 devices. // Use softfp for Android armv7 devices.
// TODO(cbracken) eliminate this when we fix https://github.com/flutter/flutter/issues/17489 // TODO(cbracken): eliminate this when we fix https://github.com/flutter/flutter/issues/17489
genSnapshotArgs.add('--no-sim-use-hardfp'); genSnapshotArgs.add('--no-sim-use-hardfp');
// Not supported by the Pixel in 32-bit mode. // Not supported by the Pixel in 32-bit mode.
......
...@@ -272,7 +272,7 @@ String getBuildDirectory() { ...@@ -272,7 +272,7 @@ String getBuildDirectory() {
/// Returns the Android build output directory. /// Returns the Android build output directory.
String getAndroidBuildDirectory() { String getAndroidBuildDirectory() {
// TODO(cbracken) move to android subdir. // TODO(cbracken): move to android subdir.
return getBuildDirectory(); return getBuildDirectory();
} }
......
...@@ -831,9 +831,9 @@ class EmulatorDomain extends Domain { ...@@ -831,9 +831,9 @@ class EmulatorDomain extends Domain {
/// This class can either: /// This class can either:
/// 1) Send stdout messages and progress events to the client IDE /// 1) Send stdout messages and progress events to the client IDE
/// 1) Log messages to stdout and send progress events to the client IDE /// 1) Log messages to stdout and send progress events to the client IDE
/// //
/// TODO(devoncarew): To simplify this code a bit, we could choose to specialize // TODO(devoncarew): To simplify this code a bit, we could choose to specialize
/// this class into two, one for each of the above use cases. // this class into two, one for each of the above use cases.
class _AppRunLogger extends Logger { class _AppRunLogger extends Logger {
_AppRunLogger(this.domain, this.app, { this.parent }); _AppRunLogger(this.domain, this.app, { this.parent });
......
...@@ -90,7 +90,7 @@ class KernelCompiler { ...@@ -90,7 +90,7 @@ class KernelCompiler {
Artifact.frontendServerSnapshotForEngineDartSdk Artifact.frontendServerSnapshotForEngineDartSdk
); );
// TODO(cbracken) eliminate pathFilter. // TODO(cbracken): eliminate pathFilter.
// Currently the compiler emits buildbot paths for the core libs in the // Currently the compiler emits buildbot paths for the core libs in the
// depfile. None of these are available on the local host. // depfile. None of these are available on the local host.
Fingerprinter fingerprinter; Fingerprinter fingerprinter;
......
...@@ -53,7 +53,7 @@ class IntelliJPlugins { ...@@ -53,7 +53,7 @@ class IntelliJPlugins {
final String jarPath = packageName.endsWith('.jar') final String jarPath = packageName.endsWith('.jar')
? fs.path.join(pluginsPath, packageName) ? fs.path.join(pluginsPath, packageName)
: fs.path.join(pluginsPath, packageName, 'lib', '$packageName.jar'); : fs.path.join(pluginsPath, packageName, 'lib', '$packageName.jar');
// TODO(danrubel) look for a better way to extract a single 2K file from the zip // TODO(danrubel): look for a better way to extract a single 2K file from the zip
// rather than reading the entire file into memory. // rather than reading the entire file into memory.
try { try {
final Archive archive = final Archive archive =
......
...@@ -13,8 +13,6 @@ import '../base/version.dart'; ...@@ -13,8 +13,6 @@ import '../base/version.dart';
const bool _includeInsiders = false; const bool _includeInsiders = false;
class VsCode { class VsCode {
static const String extensionIdentifier = 'Dart-Code.flutter';
VsCode._(this.directory, this.extensionDirectory, { Version version, this.edition }) VsCode._(this.directory, this.extensionDirectory, { Version version, this.edition })
: this.version = version ?? Version.unknown { : this.version = version ?? Version.unknown {
...@@ -52,6 +50,8 @@ class VsCode { ...@@ -52,6 +50,8 @@ class VsCode {
final Version version; final Version version;
final String edition; final String edition;
static const String extensionIdentifier = 'Dart-Code.flutter';
bool _isValid = false; bool _isValid = false;
Version _extensionVersion; Version _extensionVersion;
final List<String> _validationMessages = <String>[]; final List<String> _validationMessages = <String>[];
...@@ -118,7 +118,7 @@ class VsCode { ...@@ -118,7 +118,7 @@ class VsCode {
// Windows: // Windows:
// $programfiles(x86)\Microsoft VS Code // $programfiles(x86)\Microsoft VS Code
// $programfiles(x86)\Microsoft VS Code Insiders // $programfiles(x86)\Microsoft VS Code Insiders
// TODO: Confirm these are correct for 64bit // TODO(dantup): Confirm these are correct for 64bit
// $programfiles\Microsoft VS Code // $programfiles\Microsoft VS Code
// $programfiles\Microsoft VS Code Insiders // $programfiles\Microsoft VS Code Insiders
// Windows Extensions: // Windows Extensions:
...@@ -187,10 +187,10 @@ class VsCode { ...@@ -187,10 +187,10 @@ class VsCode {
} }
class _VsCodeInstallLocation { class _VsCodeInstallLocation {
const _VsCodeInstallLocation(this.installPath, this.extensionsFolder, { this.edition, bool isInsiders })
: this.isInsiders = isInsiders ?? false;
final String installPath; final String installPath;
final String extensionsFolder; final String extensionsFolder;
final String edition; final String edition;
final bool isInsiders; final bool isInsiders;
const _VsCodeInstallLocation(this.installPath, this.extensionsFolder, { this.edition, bool isInsiders })
: this.isInsiders = isInsiders ?? false;
} }
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